Due to the use of pthread cancel there is no way to guarantee when a thread will be killed. There have been a few drivers that have implemented work arounds for this but it continues to be an issue that pops up from time to time (quite nastily for me recently).
The attached patch changes the player driver threads to use deferred cancellation, in this way only explicit cancellation points (ProcessMessages, Wait and TestCancel). This way you can use code that holds other mutexes etc in player drivers without ending up with deadlocks.
The basic idea of the patch is to move thread initialisation into a MainSetup method instead of Setup and to make use of the MainQuit cleanup function instead of Shutdown. Additionally it rationalises the Driver class by creating a new ThreadedDriver subclass that threaded drivers should inherit from.
The patch includes updating all the player drivers in the svn tree to use the new API, plugins will need to be moved manually. The process is relatively simple (and may be able to be automated). First change your driver to use ThreadedDriver instead of Driver. Next rename the Setup method to MainSetup and Shutdown to void MainQuit. Then remove the calls to StartThread and StopThread.
While the patch is quite large, it makes it easier to correctly order the initialisation and shutdown of thread resources, a number of subtle threading issues have been fixed in the process (such as cleaning up resources before stopping the thread that uses them).
The new Start/StopThread calls (handled by the ThreadedDriver class) are asynchronous, which means the server will not block while calling a threads setup method, good for drivers that take a long time to setup. The disadvantage is that drivers cannot report resource initialisation failures via the subscribe response, however this could be handled with a new type of exception message (not included in the patch) which would be useful anyway for errors that occur during driver operation (many of which are simply printed to the console)
Given the size of the patch I would recommend that if it is wanted it is applied as soon as possible to simplify the merging.
Log in to post a comment.