> On Oct. 22, 2015, 6:23 p.m., Alan Conway wrote: > > trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp, line 645 > > <https://reviews.apache.org/r/39561/diff/1/?file=1103468#file1103468line645> > > > > We are checking pending now, but not actually processing the accept > > till the IO thread, which means the link could be deleted in between. > > Should this test be in dischargeComplete()?
The link can only be freed on the IO thread, when at th Connection level dispatch() is called before process(). It is in dispatch that the completed list is accepted, and after that in process that any link detach is handled that would free the link. So I *think* it is ok. On dischargeComplete(), I wasn't entirely clear how that is used. It has a comment saying it is called on the IO thread and always passes sync as false, yet it is called in abort() (which I think is always sync?) and in committed() only when sync is true. Calling pending_accept()/accepted() one after the other is not really right... pending_accept() should be called at the point where we know that the accept will be done asynchronously. - Gordon ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39561/#review103629 ----------------------------------------------------------- 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 > >
