[Live-devel] correct way to use TaskScheduler::unscheduleDelayedTask?
Ross Finlayson
finlayson at live555.com
Fri Aug 26 03:07:45 PDT 2016
> I caution that
> updating the documentation is insufficient, because live555 does not
> follow this rule itself.
Yes, the “fNextTask” hack is problematic, and not just because of the example that you noted. It’s also a problem because it makes the (in general, unwarranted) assumption that there will be no more than one pending delayed task that would need to be unscheduled if the “Medium” object is destroyed. So I plan to (at some point) completely replace it with a more general mechanism (which would be a significant change to the code). In the meantime, I’ll be making small ‘spot fixes’ to issues that arise with the current “fNextTask” hack (and therefore, I won’t be applying the patch that you included in your follow-up email).
> Now let us look into the BasicUDPSink subclass. Its afterGettingFrame1
> method assigns a valid TaskToken to fNextTask by calling
> scheduleDelayedTask. However, the recorded TaskFunc
> BasicUDPSink::sendNext does not update the fNextTask field, so it is now
> expired (i.e. invalid). If the object is destructed in an inopportune
> moment, the destructor runs unscheduleDelayedTask on the expired
> TaskToken, which violates your rule.
Yes, you are correct here. Fortunately, there is an easy fix (which I’ll add to the code in the near future): Set “fNext” to NULL at the start of the handler function (“sendNext()”). Ditto for the other places in the code where the same thing is done.
> I stress that this is not a theoretical problem. If one implements a
> TaskScheduler (as the faq advises) in a way that relies on this rule,
> one can reissue the same TaskToken after it has expired and thus arrive
> at double freeing memory.
Are you actually seeing these ‘double frees’ in your own application? If you are, then I’m a bit surprised, because nobody else has reported seeing these. Yes, the problem that you note is real - but for most people, it apparently doesn’t happen (because apparently - in most cases - “Medium” objects do not get destroyed between the handling/scheduling of delayed tasks). But if you’re actually seeing this happen, then it appears that your application is doing something unusual, which makes me worried, because of what you wrote next:
> I also stress that this choice makes it impossible to implement a
> TaskScheduler that actually makes unscheduleDelayedTask thread-safe.
I hope you have read the FAQ (that everyone was asked to read before posting to the mailing list :-) - specifically:
http://live555.com/liveMedia/faq.html#threads
You absolutely MUST NOT try to access a “TaskScheduler” (or any other LIVE555 object) from more than one thread. As explained in the FAQ, the LIVE555 code was designed to be run as a single-threaded event loop. (It *is* possible, however, to run LIVE555 code from more than one thread, if each thread uses its own “UsageEnvironment” and “TaskScheduler” object - again, as explained in the FAQ.)
If you are trying to access a “TaskScheduler” object from more than one thread, then you are going to run into many more problems than the “fNext” problem that you noted above. You’re going to encounter all sorts of race conditions that the code was simply not designed to deal with (because it was designed to be run within a single threaded event loop). So, you’re going to have to fix your application to stop doing this.
Ross Finlayson
Live Networks, Inc.
http://www.live555.com/
More information about the live-devel
mailing list