From: Bruno Haible Date: Thu, 30 May 2024 08:23:22 +0000 (+0200) Subject: windows-once: Free allocated resources when done. X-Git-Url: https://gitweb.git.savannah.gnu.org/gitweb/?a=commitdiff_plain;h=c6942fe973af622bbeb5fafc5c1cd32c82a2cb7b;p=gnulib.git windows-once: Free allocated resources when done. Based on an observation regarding Cygwin's pthread_once implementation by Takashi Yano at and * lib/windows-once.h (glwthread_once_t): Change type of inited to LONG. Add field 'num_threads'. (GLWTHREAD_ONCE_INIT): Initialize it to zero. * lib/windows-once.c (glwthread_once): Use InterlockedCompareExchange instead of InterlockedIncrement. Increment num_threads while the thread uses the lock. Let the last thread that uses the lock destroy it. --- diff --git a/ChangeLog b/ChangeLog index d6130f2676..72296c306e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,17 @@ +2024-05-31 Bruno Haible + + windows-once: Free allocated resources when done. + Based on an observation regarding Cygwin's pthread_once implementation + by Takashi Yano at + and + + * lib/windows-once.h (glwthread_once_t): Change type of inited to LONG. + Add field 'num_threads'. + (GLWTHREAD_ONCE_INIT): Initialize it to zero. + * lib/windows-once.c (glwthread_once): Use InterlockedCompareExchange + instead of InterlockedIncrement. Increment num_threads while the thread + uses the lock. Let the last thread that uses the lock destroy it. + 2024-05-31 Bruno Haible call_once: Work around Cygwin 3.5.3 bug. diff --git a/lib/windows-once.c b/lib/windows-once.c index 17854f5c09..4d234e0bf2 100644 --- a/lib/windows-once.c +++ b/lib/windows-once.c @@ -29,7 +29,9 @@ glwthread_once (glwthread_once_t *once_control, void (*initfunction) (void)) { if (once_control->inited <= 0) { - if (InterlockedIncrement (&once_control->started) == 0) + InterlockedIncrement (&once_control->num_threads); + /* If once_control->started is == -1, set it to 0. */ + if (InterlockedCompareExchange (&once_control->started, 0, -1) < 0) { /* This thread is the first one to come to this once_control. */ InitializeCriticalSection (&once_control->lock); @@ -41,8 +43,6 @@ glwthread_once (glwthread_once_t *once_control, void (*initfunction) (void)) } else { - /* Don't let once_control->started grow and wrap around. */ - InterlockedDecrement (&once_control->started); /* Some other thread has already started the initialization. Yield the CPU while waiting for the other thread to finish initializing and taking the lock. */ @@ -58,5 +58,46 @@ glwthread_once (glwthread_once_t *once_control, void (*initfunction) (void)) abort (); } } + /* Here once_control->inited > 0. */ + if (InterlockedDecrement (&once_control->num_threads) == 0) + /* once_control->num_threads is now zero, and once_control->inited > 0. + No other thread will need to use the lock. + We can therefore destroy the lock, to free resources. */ + /* If once_control->inited is == 1, set it to 2. */ + if (InterlockedCompareExchange (&once_control->inited, 2, 1) == 1) + DeleteCriticalSection (&once_control->lock); } + /* Proof of correctness: + * num_threads is incremented and then decremented by some threads. + Therefore, num_threads always stays >= 0, and is == 0 at the end. + * The first thread to go through the once_control->started fence + initializes the lock and moves inited from <= 0 to > 0. The other + threads don't move inited from <= 0 to > 0. + * inited, once > 0, stays > 0 (since at the place where it is assigned 0, + it cannot be > 0). + * inited does not change any more once it is 2. + Therefore, it can be changed from 1 to 2 only once. + * DeleteCriticalSection gets invoked right after inited has been changed + from 1 to 2. Therefore, DeleteCriticalSection gets invoked only once. + * After a moment where num_threads was 0 and inited was > 0, no thread can + reach an InitializeCriticalSection or EnterCriticalSection invocation. + Proof: + - At such a moment, no thread is in the code range between + InterlockedIncrement (&once_control->num_threads) + and + InterlockedDecrement (&once_control->num_threads) + - After such a moment, some thread can increment num_threads, but from + there they cannot reach the InitializeCriticalSection invocation, + because the once_control->started test prevents that, and they cannot + reach the EnterCriticalSection invocation in the other branch because + the + if (once_control->inited <= 0) + test prevents that. + * From this it follows that: + - DeleteCriticalSection cannot be executed while the lock is taken + (because DeleteCriticalSection is only executed after a moment where + num_threads was 0 and inited was > 0). + - Once DeleteCriticalSection has been executed, the lock is not used any + more. + */ } diff --git a/lib/windows-once.h b/lib/windows-once.h index c5bbcd573c..18ed8d878a 100644 --- a/lib/windows-once.h +++ b/lib/windows-once.h @@ -25,13 +25,14 @@ typedef struct { - volatile int inited; + volatile LONG inited; + volatile LONG num_threads; volatile LONG started; CRITICAL_SECTION lock; } glwthread_once_t; -#define GLWTHREAD_ONCE_INIT { -1, -1 } +#define GLWTHREAD_ONCE_INIT { -1, 0, -1 } #ifdef __cplusplus extern "C" {