From: Jonathan W. <jw...@ju...> - 2011-09-13 00:30:31
|
Hi Holger > > > There seems to be a problem with thread (un)locking. In > > > src/libieee1394/IsoHandlerManager.cpp > > > Line 1324 the application in waiting for a lock - forever. > > > > That's interesting. This lock is only taken in one place, as per the > > comment immediately above this line: in the IsoTask thread associated with > > the handler. It's not unexpected under the new stack for the IsoTask > > thread call to still be in progress when ~IsoHander() is hit, so it's > > natural that the pthread_mutex_lock() call on line 1324 will be executed. > > If this call never returns it more or less means that the > > raw1394_destroy_handle() from the IsoTask thread isn't returning. The > > only way I can think to cause this is if the handle passed to it was in > > some way corrupt, causing > > raw1394_destroy_handle() to go off into space. > > > > If this is an accurate picture of what's going on it would suggest that the > > behaviour that you're seeing is just another manifestation of issue 306. > > Since issue 306 appears at present to involve memory corruption of some > > kind this would be consistent. > > The problem is, that pthread_mutex_init is only called in one constructor - > and in the wrong one - at least in my case. Ah, you're absolutely correct. When I added this lock I forgot that the IsoHandler::IsoHandler() constructor was overloaded (which is a worry since the two other constructors follow immediately after the one I changed), so the lock is only being initialised in one specific case. To make things worse, the constructor where it *is* being initialised is not the one being called from IsoHandlerManager::registerStream(), so odds are that in normal use: 1) the lock is never being initialised, and 2) in the cases where things appear to work we've just been lucky. This has a high likelihood of resolving the residual problems in issue 306. > I have not tried thoroughly, but my first tests seems very promising... I reckon you've nailed it. In any case, adding the mutex initialisation to the other two IsoHandler constructors is obviously required anyway, so that's done now (see svn rev 1995). For completeness you might like to check out r1995 and continue your testing with that. Thanks again for providing the extra pair of eyes to this problem. I hope that with this change we have finally put issue 306 to sleep. Time and wider testing will of course tell. Regards jonathan |