> On April 25, 2013, 10:13 p.m., Alan Conway wrote: > > /trunk/qpid/cpp/src/qpid/client/amqp0_10/ReceiverImpl.cpp, line 205 > > <https://reviews.apache.org/r/10704/diff/1/?file=283074#file283074line205> > > > > It's not immediately obvious that this is safe - could there be code > > that depends on the 3 actions (release, cancel, receiverCancelled) be > > atomic? In other words is the fetchImpl in a valid state at each unlock, > > and can all other concurrent functions safely be called during the unlocks?
The releasePending(), cancel() and receiverCancelled() calls don't need to be atomic. However the setting of the state to CANCELLED would prevent concurrent interleaving of those calls since only one close request will actually proceed to execute them. The locks are primarily to ensure integrity of data structures; the receiver lock here protects the state and the session handle, the session lock will protect the receiver map and other session related structures. - Gordon ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10704/#review19736 ----------------------------------------------------------- On April 22, 2013, 3:52 p.m., Gordon Sim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10704/ > ----------------------------------------------------------- > > (Updated April 22, 2013, 3:52 p.m.) > > > Review request for qpid and Alan Conway. > > > Description > ------- > > The changes involve: > > (1) removing the unnecessary lock from ReceiverImpl::getName() since the > destination member variable is const > > (2) removing the lock from the two forms of SessionImpl:acknowledgeImpl() > since the transactional member is const and IncomingMessages::accept() does > its own locking > > (3) releasing the lock while calling back from ReceiverImpl to SessionImpl in > ReceiverImpl::close() > > These help break the possible deadlock. > > > This addresses bug QPID-4764. > https://issues.apache.org/jira/browse/QPID-4764 > > > Diffs > ----- > > /trunk/qpid/cpp/src/qpid/client/amqp0_10/ReceiverImpl.cpp 1470466 > /trunk/qpid/cpp/src/qpid/client/amqp0_10/SessionImpl.cpp 1470466 > > Diff: https://reviews.apache.org/r/10704/diff/ > > > Testing > ------- > > make check passes (I couldn't reproduce the deadlock). > > > Thanks, > > Gordon Sim > >
