[Live-devel] triggerEvent() race condition

[Robert Smith]

I would like to use boost::atomic< T> or std::atomic< T> but 
unfortunately boost and c++11 are unavailable to me. I agree that 
std::atomic< T> or the like would be the best way to go though.


On 03/18/2015 12:16 AM, Jeff Shanab wrote:
> I have used boost atomic or just a boost mutex with a scoped_lock when 
> I needed to protect, It implements cross platform the best avail on a 
> platform.
>
>
>
>
> On Tue, Mar 17, 2015 at 9:07 AM, Robert Smith <smith at cognitec.com 
> <mailto:smith at cognitec.com>> wrote:
>
>     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  <mailto:live-devel at lists.live555.com>
>>     http://lists.live555.com/mailman/listinfo/live-devel
>
>
>     _______________________________________________
>     live-devel mailing list
>     live-devel at lists.live555.com <mailto:live-devel at lists.live555.com>
>     http://lists.live555.com/mailman/listinfo/live-devel
>
>
>
>
> _______________________________________________
> 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/20150318/eedfd432/attachment.html>