 |
 |
Hi. Thanks, I didn't realise there was the specific concern that this change adds a problem. My reading of this is that:
(i) In ~Channel(), Invalidate() is called on the pTickGenerator. This removes the associated between this TickGeneratorTask and the Channel. If the Run() method of the task is called it no longer invokes any method of the Channel, and returns (time_t)0.
(ii) The TaskManager is primarily single shot; with optional re-queuing of tasks: If a task's Run() method returns non-zero it is re-queued at the time indicated by the return value. If the return value is zero the task is not re-queued, and if "owned" it is deleted. (And in this case the pTickGenerator is indeed owned by TaskManager).
(iii) Therefore after Invalidate() the task will be de-queued and deleted at some point (the next time it is scheduled to run). So currently there's a race between the Invalidate() and the UnregisterTask() in ~Channel: usually UnregisterTask() will unregister the task, but sometimes the task will have already been deleted and UnregisterTask() is called with a dangling pointer as argument.
(iv) The side-effect of removing the call to UnregisterTask() is expected to be the elimination of this race, with the side effect of keeping a task in the queue a bit longer than otherwise. However the task no longer contains an association to the channel, and will not do much when Run(), and will then be deleted.
So I'd think that removing UnregisterTask() is the appropriate, aside from issue 1883.
I haven't clearly shown this is the specific cause of issue #1883, although I could try to do so. And even if that's done it doesn't rule out the possibility of other bugs around this area; so your point about redesign here is taken. But I wanted to double check the evaluation this is change is actually introducing a bug, as that is certainly not wanted!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Use REPLY-ALL to reply to list
To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1