qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gordon Sim" <g...@redhat.com>
Subject Re: Review Request: C++ Broker normalize exchange locking and add RWlock for each
Date Mon, 11 Mar 2013 12:32:31 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9698/#review17688
-----------------------------------------------------------



trunk/qpid/cpp/src/qpid/broker/DirectExchange.cpp
<https://reviews.apache.org/r/9698/#comment37564>

    This isn't safe as the bindings map is accessed with no lock held.


- Gordon Sim


On March 8, 2013, 7:42 p.m., Chug Rolke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9698/
> -----------------------------------------------------------
> 
> (Updated March 8, 2013, 7:42 p.m.)
> 
> 
> Review request for qpid, Andrew Stitcher, Alan Conway, Gordon Sim, and Ted Ross.
> 
> 
> Description
> -------
> 
> * Move Topic exchange RWlock up into the parent exchange class and call it 'bindingLock'.
> 
> * In Topic exchange take RWlock out of the derived class and use the parent's lock.
> * In Direct and Headers exchanges change the semantics of the current lock to use the
Topic exchange pattern.
> * In FanOut exchange add locks at the same places as in the other exchanges except only
use Rlock. FanOut receives the locking it needs from the CopyOnWriteArray and does not need
anything else. By adding only Rlocks threads will never block in the normal case. However,
the Rlocks are the hooks that the new application will need to freeze the exchange when that
app takes out the Wlock.
> 
> 
> This addresses bug QPID-4616.
>     https://issues.apache.org/jira/browse/QPID-4616
> 
> 
> Diffs
> -----
> 
>   trunk/qpid/cpp/src/qpid/broker/DirectExchange.h 1454111 
>   trunk/qpid/cpp/src/qpid/broker/DirectExchange.cpp 1454111 
>   trunk/qpid/cpp/src/qpid/broker/Exchange.h 1454111 
>   trunk/qpid/cpp/src/qpid/broker/FanOutExchange.h 1454111 
>   trunk/qpid/cpp/src/qpid/broker/FanOutExchange.cpp 1454111 
>   trunk/qpid/cpp/src/qpid/broker/HeadersExchange.h 1454111 
>   trunk/qpid/cpp/src/qpid/broker/HeadersExchange.cpp 1454111 
>   trunk/qpid/cpp/src/qpid/broker/Link.cpp 1454111 
>   trunk/qpid/cpp/src/qpid/broker/TopicExchange.h 1454111 
>   trunk/qpid/cpp/src/qpid/broker/TopicExchange.cpp 1454111 
>   trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.h 1454111 
>   trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1454111 
>   trunk/qpid/cpp/src/qpid/ha/QueueReplicator.h 1454111 
>   trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1454111 
>   trunk/qpid/cpp/src/qpid/sys/CopyOnWriteArray.h 1454111 
>   trunk/qpid/cpp/src/qpid/xml/XmlExchange.h 1454111 
>   trunk/qpid/cpp/src/qpid/xml/XmlExchange.cpp 1454111 
> 
> Diff: https://reviews.apache.org/r/9698/diff/
> 
> 
> Testing
> -------
> 
> Passes normal self tests
> 
> 
> Thanks,
> 
> Chug Rolke
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message