> On May 8, 2015, at 4:30 PM, John Gates <[log in to unmask]> wrote:
>
> Hi Serge,
>
> Disclaimer: It wouldn't surprise me if one of the possible paths still leaked resources. This code is complicated. The primary problem was the normal successful operations were leaking mysql connections, of which we don't have that many to leak. This needed to be fixed quickly because tests were breaking.
Sure - I’m not arguing with any of that! I’m just trying (and failing) to understand this code.
> I see your point, and will say that when I do a '^c' to cancel a request, it isn't leaking mysql connections. It does look like there's an issue with _finishStatus that bears a closer look.
Yes and I think Jacek observed the same thing. Which is why I think I must be missing something...
> I think an easier way would be to change
>
> if (_finishStatus != ACTIVE) {
> return;
> }
>
> to:
>
> if (_finishStatus != ACTIVE) {
> if (_finishStatus == CANCELLED) {
> _cleanup();
> }
> return;
> }
>
> I see this as safe as either way we would want to get rid of those pointers at that point.
This doesn’t address the (potential?) issue with Finished() not getting called.
> Resetting the shared_ptr's at the point you do would cause the current object to be destroyed before calling Finished(true).
Are you sure? The way I see it, the current object (QueryRequest) gets deleted when cancelFunc is destructed, and cancelFunc is held by _requester, from which the cancellation must be invoked. So I think that the requester, cancelFunc, and hence QueryRequest are guaranteed to be alive while cancel() is executing.
However, I do think this issue might be there in other places. For example, _importStream calls _retryFunc, which loops back to to Executive::_queryDispatch, eventually calling registerCancel() on the requester. I think that could cause the CancelFunc holding on to the QueryReqest to be destroyed, which could delete the QueryRequest while _importStream is executing.
Cheers,
Serge
########################################################################
Use REPLY-ALL to reply to list
To unsubscribe from the QSERV-L list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=QSERV-L&A=1
