GNOME Bugzilla – Bug 737851
Critical sections not released correctly.
Last modified: 2015-03-03 11:49:39 UTC
Created attachment 287687 [details] Patch for the xmlRMutexUnlock() function in threads.c In threads.c the xmlRMutexLock() and xmlRMutexUnlock() functions will use critical sections on windows. Every call to the xmlRMutexLock() function will result in a call to EnterCriticalSection() and will increment the mutex count variable. The xmlRMutexUnlock() function will then decrement the mutex count variable, and only call LeaveCriticalSection when the count is down to zero. According to the Microsoft documentation this is incorrect: (http://msdn.microsoft.com/en-ca/library/windows/desktop/ms682608%28v=vs.85%29.aspx) "A thread must call LeaveCriticalSection once for each time that it entered the critical section."
Hum, your patch being reversed and without context I started getting a bit confused, but okay, I think I fixed it: https://git.gnome.org/browse/libxml2/commit/?id=8854e4631844eac8dbae10cc32904f27d5268af7 please check and report, this should make the upcoming libxml2-2.9.2 release so feedback rather urgent, thanks ! Daniel
Unfortunately this change has introduced a problem which results in occasional hangs on Windows when running multi-threaded on a multi-core host. When locking the xmlRMutex the count field is increment inside the critical section but when unlocking the count field is decremented outside the critical section. The increment/decrement is not atomic so this can result in the count field being updated incorrectly. The solution is to change xmlRMutexUnlock to decrement the count field before leaving the critical section rather than after: ---- void xmlRMutexUnlock(xmlRMutexPtr tok) { if (tok->count > 0) { tok->count--; LeaveCriticalSection(&tok->cs); } } ---- Cheers, Steve Nairn
Indeed that's broken ! Pushed, https://git.gnome.org/browse/libxml2/commit/?id=620a70615e68b30db1a80a993180a41dc24f12b9 thanks for the head-up ! Daniel