[Live-devel] triggerEvent() race condition

[Robert Smith]

Ok, I will implement my own TaskScheduler class, but operations that 
read-modify-write memory are not atomic with multiple processors/cores. 
I guess the large portion of users these days will be using multicore 
x86 processors.

I asked this question on stackoverflow and the responses support the 
research I have done myself.

http://stackoverflow.com/questions/29099094/is-the-c-operator-atomic-with-a-multicore-processor

My updated  application with a protected triggerEvent() method didn't 
cure the problem, one (out of six) of the streams stopped after an hour. 
I then modified BasicTaskScheduler to protect all reads/writes of the 
fTriggersAwaitingHandling and it has been running for over 24 hours now. 
Time will tell but it looks promising.

If I'm to speculate why this hasn't been a problem before I would guess 
that the large majority of users are not using the triggers at runtime, 
e.g, they are using the file sources. And for those who are using a live 
source, if they only have one stream then the task schedulers' 
SingleStep() method is probably waiting in the select() call when the 
triggerEvent() method is called making it unlikely that they conflict.

Also each thread in my application has it's own event trigger id.

I'm not even suggesting that you change the implementation but at least 
be aware of the problem and document it for future users.


On 03/16/2015 05:50 PM, Ross Finlayson wrote:
>> Having thought about it some more, I don't think it's enough to just 
>> protect calls to triggerEvent() because 'fTriggersAwaitingHandling' 
>> is still modified by BasicTaskScheduler::SingleStep().
>
> The intention of the code that implements "BasicTaskScheduler" was that:
> 1/ The event loop code (which is always just a single thread, 
> remember) does nothing with "fTriggersAwaitingHandling" except clear 
> (using "&=~") a single bit that was already set, and
> 2/ The other thread(s) that can trigger event(s) do nothing 
> with "fTriggersAwaitingHandling" except set (using "|=") bit(s) that 
> were not already set.
>
> If these two conditions are met, *and* if the 'clear a bit' and 'set a 
> bit' operations are atomic, then the code will be thread-safe.
>
> (If, OTOH, you're worried that the 'clear a bit' and 'set a bit' 
> operations might not be atomic on your system, then you'd need to 
> write your own "TaskScheduler" subclass instead.)
>
> Looking at the code again just now, however, I realized that the event 
> loop code does not always satisfy condition 1/.   Line 183 of 
> "BasicTaskScheduler.cpp" should be changed from:
> fTriggersAwaitingHandling = 0;
> to
> fTriggersAwaitingHandling &=~ fLastUsedTriggerMask;
>
> I have just released a new version (2015.03.16) of the code that fixes 
> this.  You should upgrade to this version.
>
>
> You should also make sure that your own code satisfies condition 2/. 
>  I.e., you should make sure that your code is never calling 
> "triggerEvent()" more than once with the same 'event trigger' value, 
> unless you're sure that the first event (for that event trigger) has 
> already been handled.  (In particular, you should never call 
> "triggerEvent()" with the same 'event trigger' value from more than 
> one thread.)
>
> Ross Finlayson
> Live Networks, Inc.
> http://www.live555.com/
>
>
>
>
> _______________________________________________
> live-devel mailing list
> live-devel at lists.live555.com
> http://lists.live555.com/mailman/listinfo/live-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.live555.com/pipermail/live-devel/attachments/20150317/75f3b168/attachment.html>