Opened 7 years ago
Closed 7 years ago
#175 closed defect (fixed)
pthread: remove DosEnterCritSec usage
Reported by: | dmik | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | |
Component: | *none | Version: | |
Severity: | highest | Keywords: | |
Cc: |
Description
It seems that at least pthread_detach
and pthread_kill
use DosEnterCritSec
. This is absolutely not necessary there, and more over, it's very dangerous as it's a frequent cause of hard system deadlocks.
Change History (3)
comment:1 by , 7 years ago
comment:2 by , 7 years ago
Severity: | medium → highest |
---|
comment:3 by , 7 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Removed all DosEnterCritSec
usage in r2250. Note that in the detach
and kill
code it was removed at all w/o any replacement so, as opposed to pthread-win32, no serialization is necessary there (see https://github.com/GerHobbelt/pthread-win32/blob/master/ptw32_reuse.c for details on why it's needed there).
In other places it was replaced with LIBC _smutex
which is very similar in that it requires no creation of a mutex (it only needs a variable intialized with zero). The first S in _smutex
stands for "simple" because it is implemented as a trivial spin loop (atomic check and DosSleep in a loop) so it should be used only in cases where a collision between two or more threads is unlikely. This is exactly what DosEnterCritSec
was used for. So _smutex
is generally a good replacement for the OS/2 critical section. And if a collision is likely, then _fmutex
should be used (in kLIBC it also has an form that doesn't require implicit mutex creation, using the FMUTEX_INITIALIZER_EX macro).
Closing this.
It was originally discovered while debugging Python hangs, see http://trac.netlabs.org/rpm/ticket/276#comment:2 for more info.
The source code for
pthread_detach
andpthread_kill
(introduced by r234) was taken from pthread-win32, see e.g. https://github.com/GerHobbelt/pthread-win32/blob/master/pthread_detach.c and https://github.com/GerHobbelt/pthread-win32/blob/master/pthread_kill.c. Older versions of these functions used Win32EnterCriticalSection
(see e.g. https://github.com/GerHobbelt/pthread-win32/blame/eb28d5aa00690b67cc27e4bd93d2c4c251987c85/pthread_detach.c). And while it looks thatDosEnterCritSec
is its OS/2 counterpart, it's not — it's a completely different thing (although such a misuse is not that rare).EnterCriticalSection
is merely a lighter version of a regular mutex which requires less kernel calls whileDosEnterCritSec
is not a mutex, it's a process global lock that disables thread switching for the current process (i.e. it suspends all other threads except the current one indefinitely).Newer versions of pthread-win32 use a custom EnterCriticalSection replacement (for whatever reason; optimization for modern CPUs most likely). We can either port this replacement to OS/2 or simply use LIBC fast mutex instead. I vote for the latter as it's faster to implement (and these mutexes are quite fast too, so I don't think we'll gain much after porting the pthread-win32 implementation).