> On Oct. 22, 2015, 7:29 p.m., Kenneth Giusti wrote: > > trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp, line 645 > > <https://reviews.apache.org/r/39561/diff/1/?file=1103468#file1103468line645> > > > > There's a teensy weensy race here: it is possible that accepted() will > > be called with sync=True even when clone() has been called (as all > > completers may finish while the clone() is executing). So the delivery will > > also be sitting in the pending set and gets re-settled on detach. > > > > > > Maybe the deliveries should be put on / removed from the pending set > > unconditionally?
The pending set doesn't of itself cause any completions. What would happen in the case you describe is that the delivery would be stuck in pending with not async accepted call ever coming to clear it out. If it is indeed the case that sync may be true even after clone(), then the sync=true path for accepted could erase the delivery from pending if it is there. - Gordon ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39561/#review103641 ----------------------------------------------------------- On Oct. 22, 2015, 5:37 p.m., Gordon Sim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39561/ > ----------------------------------------------------------- > > (Updated Oct. 22, 2015, 5:37 p.m.) > > > Review request for qpid, Alan Conway and Kenneth Giusti. > > > Bugs: QPID-6790 > https://issues.apache.org/jira/browse/QPID-6790 > > > Repository: qpid > > > Description > ------- > > This is an alterntaive fix that simply shortcircuits the asynchornous > completion for deliveries whose link has already been freed. It does this by > tracking deliveries for which asynchronous acceptance is pending, and on > deleting links removes the records of any deliveries affected. On handling an > asynchronous accept, the epnding set is consulted first and the accept only > proceeds if it is found. > > > Diffs > ----- > > trunk/qpid/cpp/src/qpid/broker/amqp/Incoming.cpp 1710066 > trunk/qpid/cpp/src/qpid/broker/amqp/Session.h 1710066 > trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp 1710066 > > Diff: https://reviews.apache.org/r/39561/diff/ > > > Testing > ------- > > Passes make test. > > > Thanks, > > Gordon Sim > >
