[Sc-devel] [bug] possible race condition in server between SC_AudioDriver::RunThread()and ProcessOSCPacket()

["Ross Bencina" <rossb-lists@xxxxxxxxxxxxxx>]

Hi Guys

Sorry for the possible repost, I'm not sure whether this got through last 
week...

Look what I found...

ProcessOSCPacket() (SC_CoreAudio.cpp, line 188) can enqueue new OSC packets
onto the mToEngine FIFO from any thread (potentially ProcessOSCPacket() can
be called simultaneously from a UDP and TCP sender for example). For this
reason is locks the driver lock using: inWorld->mDriverLock->Lock(); All
good so far...

If you search the code, no one else ever locks mDriverLock, but there other
writers to the mToEngine FIFO. This would be any callers of
SendMsgToEngine(), eg SC_SequencedCommand and callers of SendMsgToRT via
InterfaceTable.

At a minimum I expected to see mDriverLock locked in
SC_AudioDriver::RunThread() (SC_CoreAudio.cpp line 374) where messages in
the mFromEngine FIFO are performed, since they are likely to result in
SC_SequencedCommand calling SendMsgToEngine().

The only explanation I can come up with for this race being encountered so
rarely is that SC_AudioDriver::RunThread() will run at very high priority
(at least on uniprocessor OSX) and hence won't be preempted while writing to
the mToEngine FIFO. However given that SC_CoreAudio.cpp line 310 reads:
#pragma message("$$$todo fixme hack for the 'uninitialized packet->mData ptr
when using MSVC 7.1 debug")
I am suspicious that this problem is being hit, at least on Windows.

Assuming others agree there is a problem, I think there are a few possible
solutions:

#1, wrap the call to mFromEngine.Perform(); in SC_AudioDriver::RunThread()
with calls to lock/unlock:
mWorld->mDriverLock->Lock();
mFromEngine.Perform();
mWorld->mDriverLock->Unlock();

#2, move the locking into SendMsgToEngine. This would mean one lock/unlock
for every SC_SequencedCommand, even if there were multiple in one NRT cycle,
however this would fix potential bugs when SendMsgToRT is called via
InterfaceTable.

#3 get rid of mDriverLock and use mNRTLock in ProcessOSCPacket() -- I have
no idea what other side effects this would have.

#4 use a separate FIFO for sending OSC packets to the RT Engine. This would
mean zero interference with the scheduling of the NRT thread, and wouldn't
add much additional code (although it would lead to allocating another FIFO
buffer)...

I think the last option (option #4) is probably the best. Thoughts?


Cheers

Ross.