> 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/
_______________________________________________
live-devel mailing list
live-devel@lists.live555.com
http://lists.live555.com/mailman/listinfo/live-devel
