[QPID-3877] Modifying a message's headers may cause a broker crash.

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 0.16
• Fix Version/s: 0.16
• Component/s: C++ Broker
• Assignee: Ken Giusti
• Reporter: Ken Giusti
• Created: 01/Mar/12 21:52
• Updated: 29/Jul/13 18:53
• Resolved: 20/Mar/12 18:35

Description

Adjusting the message's TTL header can cause a crash if another thread is attempting to encode the same message. The below trace is from valgrind while the store is in use:

==15071== Thread 4:
==15071== Invalid read of size 8
==15071== at 0x52661F0: qpid::framing::DeliveryProperties::bodySize() const (DeliveryProperties.cpp:198)
==15071== by 0x5266248: qpid::framing::DeliveryProperties::encodedSize() const (DeliveryProperties.cpp:209)
==15071== by 0x52CC3D4: qpid::framing::AMQHeaderBody::encodedSize() const (AMQHeaderBody.h:45)
==15071== by 0x52CAFEE: qpid::framing::AMQFrame::encodedSize() const (AMQFrame.cpp:46)
==15071== by 0x4DB3B55: qpid::broker::Message::encodedHeaderSize() const (frame_functors.h:39)
==15071== by 0x5C1FBFC: mrg::msgstore::MessageStoreImpl::msgEncode(std::vector<char, std::allocator<char> >&, boost::intrusive_ptr<qpid::broker::PersistableMessage> const&) (Messag\
eStoreImpl.cpp:1311)
==15071== by 0x5C31E8D: mrg::msgstore::MessageStoreImpl::store(qpid::broker::PersistableQueue const*, mrg::msgstore::TxnCtxt*, boost::intrusive_ptr<qpid::broker::PersistableMessage\
> const&, bool) (MessageStoreImpl.cpp:1331)
==15071== by 0x5C32A0B: mrg::msgstore::MessageStoreImpl::enqueue(qpid::broker::TransactionContext*, boost::intrusive_ptr<qpid::broker::PersistableMessage> const&, qpid::broker::Per\
sistableQueue const&) (MessageStoreImpl.cpp:1303)
==15071== by 0x4DBE65F: qpid::broker::MessageStoreModule::enqueue(qpid::broker::TransactionContext*, boost::intrusive_ptr<qpid::broker::PersistableMessage> const&, qpid::broker::Pe\
rsistableQueue const&) (MessageStoreModule.cpp:125)
==15071== by 0x4DCFF31: qpid::broker::Queue::enqueue(qpid::broker::TransactionContext*, boost::intrusive_ptr<qpid::broker::Message>&, bool) (Queue.cpp:811)
==15071== by 0x4DD1951: qpid::broker::Queue::deliver(boost::intrusive_ptr<qpid::broker::Message>) (Queue.cpp:171)
==15071== by 0x4D798DE: qpid::broker::DeliverableMessage::deliverTo(boost::shared_ptr<qpid::broker::Queue> const&) (DeliverableMessage.cpp:33)
==15071== Address 0xca1d548 is 56 bytes inside a block of size 248 free'd
==15071== at 0x4A0545F: operator delete(void*) (vg_replace_malloc.c:387)
==15071== by 0x52CB195: qpid::framing::AMQFrame::cloneBody() (RefCounted.h:42)
==15071== by 0x4DB37DE: qpid::broker::Message::getHeaderBody() (Message.cpp:351)
==15071== by 0x4DB7471: qpid::framing::DeliveryProperties* qpid::broker::Message::getModifiableProperties<qpid::framing::DeliveryProperties>() (Message.h:208)
==15071== by 0x4DB65E7: qpid::broker::Message::adjustTtl() (Message.cpp:416)
==15071== by 0x4D7BB60: qpid::broker::DeliveryRecord::deliver(qpid::framing::Handler<qpid::framing::AMQFrame&>&, qpid::framing::SequenceNumber, unsigned short) (DeliveryRecord.cpp:\
80)
==15071== by 0x4E1A9B9: qpid::broker::SessionState::deliver(qpid::broker::DeliveryRecord&, bool) (SessionState.cpp:380)
==15071== by 0x4DFD345: qpid::broker::SemanticState::ConsumerImpl::deliver(qpid::broker::QueuedMessage&) (SemanticState.cpp:342)
==15071== by 0x4DD519C: qpid::broker::Queue::dispatch(boost::shared_ptr<qpid::broker::Consumer>) (Queue.cpp:393)
==15071== by 0x4E00759: qpid::broker::SemanticState::ConsumerImpl::doOutput() (SemanticState.cpp:741)
==15071== by 0x52F174C: qpid::sys::AggregateOutput::doOutput() (AggregateOutput.cpp:59)
==15071== by 0x4D6FC18: qpid::broker::Connection::doOutput() (Connection.cpp:354)
==15071==

Gordon correctly points out that the message lock must be held while the headers are being encoded.