Comments below:
On 5/8/2015 5:43 PM, Serge Monkewitz wrote:
> 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.
As soon as the reference count goes to zero, _requester is deleted,
which calls canceller's destructor, which deletes this QueryRequest. The
odds of QueryRequest having the last two references to _requester are
pretty good. Further, I think cancel can be called by another thread and
having QueryRequest get deleted before xrootd calls ProcessResponseData
would be bad.
I've found that setting a flag to cancel/terminate a thread in the
target thread is usually the safest course of action. The behavior is
much more predictable. I also like sticking all the termination code in
one function as duplication of the code can cause issues that are
difficult to detect.
It looks like nobody is calling XrdSsiRequest::Finished(), at least not
inside of QueryRequest. I think we should look into that.
> 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.
The retry code scares me.
Have a good weekend,
John
########################################################################
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
|
