You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Gordon Sim <gs...@redhat.com> on 2015/10/22 19:37:49 UTC
Review Request 39561: prevent asynchornous acceptance of incoming
deliveries after thier associated link has been freed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39561/
-----------------------------------------------------------
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
Re: Review Request 39561: prevent asynchornous acceptance of incoming
deliveries after thier associated link has been freed
Posted by Gordon Sim <gs...@redhat.com>.
> 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
>
>
Re: Review Request 39561: prevent asynchornous acceptance of incoming
deliveries after thier associated link has been freed
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39561/#review103641
-----------------------------------------------------------
trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp (line 645)
<https://reviews.apache.org/r/39561/#comment161704>
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?
- Kenneth Giusti
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
>
>
Re: Review Request 39561: prevent asynchornous acceptance of incoming
deliveries after thier associated link has been freed
Posted by Gordon Sim <gs...@redhat.com>.
> On Oct. 23, 2015, 12:55 p.m., Kenneth Giusti wrote:
> > trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp, line 663
> > <https://reviews.apache.org/r/39561/diff/3/?file=1103979#file1103979line663>
> >
> > Sorry for being dense - my caffine level isn't quite where is should be - but if this test passes, isn't the delivery on both the pending and the completed containers?
Yes. If then as Alan had hypothesised, the link gets deleted before we process completed, the delivery will have been removed from pending, so when we come to act on the record in completed we see its no longer in pending and don't proceed.
On Oct. 23, 2015, 12:55 p.m., Gordon Sim wrote:
> > I still have remaining concerns regarding the lifecycle management of pn_link objects. If these seem legit, perhaps they should be tracked in another jira:
> >
> > There are several places where pn_link_closed is called by the Connection processing code due to exception handling, eg:
> >
> > https://github.com/apache/qpid/blob/trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp#L594
> > https://github.com/apache/qpid/blob/trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp#L665
> > https://github.com/apache/qpid/blob/trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp#L594
> >
> > Yet the only call to pn_link_free is here:
> > https://github.com/apache/qpid/blob/trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp#L634
> >
> > The attach/readable/writable Session methods never catch the exception, so I wonder if there is a pn_link_t leak here, or at least the possibility that the related Session/link state is ambiguous and will bite us later. Should the Session trap link-related exceptions and handle cleanup instead?
> >
> > My original patch moved the 'ownership' of the pn_link resource into the Incoming/Outgoing objects, which were tasked with calling pn_link_free (or probably pn_decref might be better?) on destruction. That RAII pattern seems more foolproof with regard to freeing link resources.
> >
> >
> > 2) I'm not convinced this detach processing is legit:
> >
> > https://github.com/apache/qpid/blob/trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp#L624
> >
> > The problem is that qpidd immediately frees the pn_link at the end of the method. From reading the AMQP spec, it seems that qpidd should only free the pn_link after it has sent a detach w/close=true. Sending a detach with close=false indicates the link is still present (though detached) - freeing the link when it is in that state contradicts my interpretation of the spec.
If the broker initiates the detach, then the client should respond by also detaching, which should then result in doLinkRemoteDetach() being called which would then free the link.
On the second point, the pn_link_t object is not the same as the logical link, so freeing it locally doesn't imply anything other than that the broker is finished with that object and won't use it again. If the client then reattached using the same link name and container id (perhaps on a different connection) a new pn_link_t would be created to represent that link, even if then associated with some other broker resident terminus state.
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39561/#review103760
-----------------------------------------------------------
On Oct. 23, 2015, 9:46 a.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39561/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2015, 9:46 a.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
>
>
Re: Review Request 39561: prevent asynchornous acceptance of incoming
deliveries after thier associated link has been freed
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39561/#review103760
-----------------------------------------------------------
trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp (line 663)
<https://reviews.apache.org/r/39561/#comment161840>
Sorry for being dense - my caffine level isn't quite where is should be - but if this test passes, isn't the delivery on both the pending and the completed containers?
I still have remaining concerns regarding the lifecycle management of pn_link objects. If these seem legit, perhaps they should be tracked in another jira:
There are several places where pn_link_closed is called by the Connection processing code due to exception handling, eg:
https://github.com/apache/qpid/blob/trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp#L594
https://github.com/apache/qpid/blob/trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp#L665
https://github.com/apache/qpid/blob/trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp#L594
Yet the only call to pn_link_free is here:
https://github.com/apache/qpid/blob/trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp#L634
The attach/readable/writable Session methods never catch the exception, so I wonder if there is a pn_link_t leak here, or at least the possibility that the related Session/link state is ambiguous and will bite us later. Should the Session trap link-related exceptions and handle cleanup instead?
My original patch moved the 'ownership' of the pn_link resource into the Incoming/Outgoing objects, which were tasked with calling pn_link_free (or probably pn_decref might be better?) on destruction. That RAII pattern seems more foolproof with regard to freeing link resources.
2) I'm not convinced this detach processing is legit:
https://github.com/apache/qpid/blob/trunk/qpid/cpp/src/qpid/broker/amqp/Connection.cpp#L624
The problem is that qpidd immediately frees the pn_link at the end of the method. From reading the AMQP spec, it seems that qpidd should only free the pn_link after it has sent a detach w/close=true. Sending a detach with close=false indicates the link is still present (though detached) - freeing the link when it is in that state contradicts my interpretation of the spec.
- Kenneth Giusti
On Oct. 23, 2015, 9:46 a.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39561/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2015, 9:46 a.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
>
>
Re: Review Request 39561: prevent asynchornous acceptance of incoming
deliveries after thier associated link has been freed
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39561/#review103769
-----------------------------------------------------------
Ship it!
Ship It!
- Kenneth Giusti
On Oct. 23, 2015, 9:46 a.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39561/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2015, 9:46 a.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
>
>
Re: Review Request 39561: prevent asynchornous acceptance of incoming
deliveries after thier associated link has been freed
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39561/#review104071
-----------------------------------------------------------
Ship it!
Ship It!
- Alan Conway
On Oct. 23, 2015, 9:46 a.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39561/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2015, 9:46 a.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
>
>
Re: Review Request 39561: prevent asynchornous acceptance of incoming
deliveries after thier associated link has been freed
Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39561/
-----------------------------------------------------------
(Updated Oct. 23, 2015, 9:46 a.m.)
Review request for qpid, Alan Conway and Kenneth Giusti.
Changes
-------
Small addition to previous update, ensuring that the disharge message is marked as pending.
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 (updated)
-----
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
Re: Review Request 39561: prevent asynchornous acceptance of incoming
deliveries after thier associated link has been freed
Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39561/
-----------------------------------------------------------
(Updated Oct. 23, 2015, 9:28 a.m.)
Review request for qpid, Alan Conway and Kenneth Giusti.
Changes
-------
Based on the clarification from Ken, that a call to clone() does not imply async completion in all cases, I have updated the patch to *always* record a pending accept whether or not that accept is then made synchronously or triggered by an asynchronous callback. The accepted() call now clears the epnding status when actually accepting, i.e. when sync=true. When sync=false, accepted() merely checks that the delivery is indeed pending before adding it to the completed set, which I think gives a clearer guarantee against the problem Alan raised.
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 (updated)
-----
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
Re: Review Request 39561: prevent asynchornous acceptance of incoming
deliveries after thier associated link has been freed
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39561/#review103654
-----------------------------------------------------------
Ship it!
Aside from the sync-race, this patch does indeed seem to fix the crash. I've tested it against a machine that would crash consistently, and with the patch there is no longer a crash and the interop tests pass.
- Kenneth Giusti
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
>
>
Re: Review Request 39561: prevent asynchornous acceptance of incoming
deliveries after thier associated link has been freed
Posted by Gordon Sim <gs...@redhat.com>.
> 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
>
>
Re: Review Request 39561: prevent asynchornous acceptance of incoming
deliveries after thier associated link has been freed
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39561/#review103629
-----------------------------------------------------------
trunk/qpid/cpp/src/qpid/broker/amqp/Session.cpp (line 645)
<https://reviews.apache.org/r/39561/#comment161690>
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()?
- Alan Conway
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
>
>