rsprin...@mail.etinternational.com wrote:
All,
We've been using Qpid M4 and 0.5 internally, both with C++, and have been
seeing occasional segfaults within the broker.  They all seem to occur
inside TopicExchange::isBound() (the one with 3 parameters, not 2).

Looking at the access patterns in this function, and judging by the
core dumps, it appears that multiple threads may access and modify
the class member "bindings" simultaneously, which seems like it
has the potential to cause a segfault.

Thanks very much for the report! I think you are exactly right, the topic exchange has no locking on the isBound() check and is therefore unsafe.

I raised a JIRA: https://issues.apache.org/jira/browse/QPID-1963

Since this is a race condition (if we're right), it's hard to
say for sure that the problem has been resolved, but since doing
the below, we've not had a segfault (roughly 5 hours so far,
rather than every 2-3 hours or so).
 - We added locks to all accesses of bindings throughout the
   file.  I believe the new ones were limited to isBound.
 - This may not be necessary at all, but to be safe (while
   we were investigating the problem), we changed all lock
   types from RWlock::ScopedLocks to Mutex::ScopedLocks, to avoid
   being tripped up by pthread (read/write) mutex semantics.

My suggested fix would be to simply hold a read lock across the isBound() method (as per attached patch). I have a reproducer that causes a crash within a few minutes and will try this fix out with it (so far so good).

We wanted to get the opinion of the experienced Qpid developers
on this list before opening a bug, but again, so far it seems to
be much more stable (we're hammering on Session::exchangeBound()
in our testing, which may be why we're seeing it in the first place).

I'm not sure if the other exchange types have the same problem or not.

From a brief examination, the other types do appear to have locking for that method. I'll run my test against them as well however.

Thanks!
-Rob

Thanks again for raising the issue and for all the investigation you have done on this so far!

--Gordon.
Index: src/qpid/broker/TopicExchange.cpp
===================================================================
--- src/qpid/broker/TopicExchange.cpp	(revision 789671)
+++ src/qpid/broker/TopicExchange.cpp	(working copy)
@@ -322,6 +322,7 @@
 
 bool TopicExchange::isBound(Queue::shared_ptr queue, const string* const routingKey, const FieldTable* const)
 {
+    RWlock::ScopedRlock l(lock);
     if (routingKey && queue) {
         string key(normalize(*routingKey));
         return isBound(queue, key);

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscr...@qpid.apache.org

Reply via email to