Bug 741516

Summary: broker cores under federation script
Product: Red Hat Enterprise MRG Reporter: mick <mgoulish>
Component: qpid-cppAssignee: mick <mgoulish>
Status: CLOSED CURRENTRELEASE QA Contact: MRG Quality Engineering <mrgqe-bugs>
Severity: urgent Docs Contact:
Priority: unspecified    
Version: DevelopmentCC: jross
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: 0.14 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-12-10 21:21:15 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Attachments:
Description Flags
reproducing environment
none
improved patch none

Description mick 2011-09-27 06:36:29 UTC
A simple federation test causes primary broker to core with 100% repeatability.

trunk svn rev 1173156

Attached archive contains reproducing environment.

Here is the core backtrace: 

    #0  0x00002aaab4000262 in ?? ()
    #1  0x00000032cbabdb91 in __dynamic_cast () from /usr/lib64/libstdc++.so.6
    #2  0x00002b07e0c7833b in polymorphic_downcast<qpid::framing::AMQHeaderBody const*, qpid::framing::AMQBody const> (this=<value optimized out>)
        at /usr/include/boost/cast.hpp:95
    #3  castBody<qpid::framing::AMQHeaderBody> (this=<value optimized out>)
        at ./qpid/framing/AMQFrame.h:59
    #4  qpid::framing::FrameSet::getHeaders (this=<value optimized out>)
        at qpid/framing/FrameSet.cpp:72
    #5  0x00002b07e073646c in qpid::broker::TransferAdapter::isPersistent (
        this=<value optimized out>, f=...) at qpid/broker/MessageAdapter.cpp:60
    #6  0x00002b07e07351f9 in qpid::broker::Message::isPersistent (
        this=0x2aaab41714f0) at qpid/broker/Message.cpp:108
    #7  0x00002b07e074cbd1 in mgntDeqStats (this=0x548c170, msg=...)
        at ./qpid/broker/Queue.h:170
    #8  qpid::broker::Queue::dequeued (this=0x548c170, msg=...)
        at qpid/broker/Queue.cpp:711
    #9  0x00002b07e074d8c7 in qpid::broker::Queue::dequeue (this=0x548c170,
        ctxt=0x0, msg=...) at qpid/broker/Queue.cpp:666
    #10 0x00002b07e06f1aac in qpid::broker::DeliveryRecord::accept (
        this=0x54840e0, ctxt=0x0) at qpid/broker/DeliveryRecord.cpp:115
    #11 0x00002b07e07924dd in operator() (__first=..., __last=..., __pred=...)
        at /usr/include/boost/bind/mem_fn_template.hpp:159

Comment 1 mick 2011-09-27 07:01:40 UTC
Proposed patch:

( Thanks, Gordon. )

Index: src/qpid/broker/Message.cpp
===================================================================
--- src/qpid/broker/Message.cpp (revision 1173156)
+++ src/qpid/broker/Message.cpp (working copy)
@@ -105,6 +105,8 @@

 bool Message::isPersistent() const
 {
+    sys::Mutex::ScopedLock l(lock);
+
     return (getAdapter().isPersistent(frames) || forcePersistentPolicy);
 }

Comment 2 mick 2011-09-27 07:33:41 UTC
Created attachment 525044 [details]
reproducing environment

look at the README file -- 

this reproducer has a couple local executables which it builds, but it also depends on a qpid source tree, which it points to with an environment variable -- you can edit that appropriately in the included scripts.

Comment 3 mick 2011-09-28 13:52:22 UTC
Created attachment 525342 [details]
improved patch

(see attachment  "improved patch" )


Message::isPersistent() is not the only function that needs to hold the lock

It needs to hold the lock because it accesses the FrameSet header, but it does that through a fn in TransferAdapter.

What other fns in Message do likewise?

  fns in TransferAdapter that call getHeaders
  {
    getApplicationHeaders
    isPersistent
    getPriority
    getAppId
  }

  fns in Message that call one of those ^^^
  {
    Message::getApplicationHeaders
    Message::isExcluded
    Message::addTraceId               ( already had lock )
    Message::insertCustomProperty*    ( already had lock )
    Message::removeCustomProperty     ( already had lock )
    Message::forcePersistent
    Message::isPersistent
    Message::getPriority              ( already had lock )
    Message::getAppId
  }

Comment 4 mick 2011-09-29 20:22:58 UTC
related to:  QPID-3304

fix in rev 1177412.