Re: [PATCH v2] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

2024-06-02 Thread Takashi Yano via Cygwin
On Sun, 02 Jun 2024 15:14:51 +0200 Bruno Haible wrote: > Hi Takashi Yano, > > > The result is as follows (submitted as v4 patch). > > > > int > > pthread::once (pthread_once_t *once_control, void (*init_routine) (void)) > > { > > /* Sign bit of once_control->state is used as done flag. > >

Re: [PATCH v2] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

2024-06-02 Thread Bruno Haible via Cygwin
Hi Takashi Yano, > The result is as follows (submitted as v4 patch). > > int > pthread::once (pthread_once_t *once_control, void (*init_routine) (void)) > { > /* Sign bit of once_control->state is used as done flag. > Similary, the next significant bit is used as destroyed flag. */ >

Re: [PATCH v2] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

2024-06-01 Thread Takashi Yano via Cygwin
On Sat, 1 Jun 2024 12:08:51 -0400 Ken Brown wrote: > Hi Takashi, > > On 6/1/2024 10:18 AM, Takashi Yano via Cygwin wrote: > > int > > pthread::once (pthread_once_t *once_control, void (*init_routine) (void)) > > { > >/* Sign bit of once_control->state is used as done flag. > > Similary,

Re: [PATCH v2] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

2024-06-01 Thread Ken Brown via Cygwin
Hi Takashi, On 6/1/2024 10:18 AM, Takashi Yano via Cygwin wrote: int pthread::once (pthread_once_t *once_control, void (*init_routine) (void)) { /* Sign bit of once_control->state is used as done flag. Similary, the next significant bit is used as destroyed flag. */ Typo:

Re: [PATCH v2] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

2024-06-01 Thread Takashi Yano via Cygwin
Hi Bruno, On Fri, 31 May 2024 16:01:35 +0200 Bruno Haible wrote: > Hi Takashi Yano, > > > With v3 patch: > > int > > pthread::once (pthread_once_t *once_control, void (*init_routine) (void)) > > { > > /* Sign bit of once_control->state is used as done flag */ > > if (once_control->state &

Re: [PATCH v2] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

2024-05-31 Thread Bruno Haible via Cygwin
Hi Takashi Yano, > With v3 patch: > int > pthread::once (pthread_once_t *once_control, void (*init_routine) (void)) > { > /* Sign bit of once_control->state is used as done flag */ > if (once_control->state & INT_MIN) > return 0; > // HERE: Point A. > /* The type of

Re: [PATCH v2] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

2024-05-30 Thread Bruno Haible via Cygwin
Takashi Yano wrote in cygwin-patches: > With v3 patch: > int > pthread::once (pthread_once_t *once_control, void (*init_routine) (void)) > { > /* Sign bit of once_control->state is used as done flag */ > if (once_control->state & INT_MIN) > return 0; > > /* The type of _control->state

Re: [PATCH v2] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

2024-05-30 Thread Takashi Yano
On Thu, 30 May 2024 20:50:12 +0900 Takashi Yano wrote: > On Thu, 30 May 2024 12:14:10 +0200 > Bruno Haible wrote: > > Takashi Yano wrote in cygwin-patches: > > > int > > > pthread::once (pthread_once_t *once_control, void (*init_routine) (void)) > > > { > > > - // already done ? > > > - if

Re: [PATCH v2] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

2024-05-30 Thread Takashi Yano
On Thu, 30 May 2024 12:14:10 +0200 Bruno Haible wrote: > Takashi Yano wrote in cygwin-patches: > > int > > pthread::once (pthread_once_t *once_control, void (*init_routine) (void)) > > { > > - // already done ? > > - if (once_control->state) > > + /* Sign bit of once_control->state is used

Re: [PATCH v2] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

2024-05-30 Thread Noel Grandin via Cygwin
On 5/30/2024 11:15 AM, Bruno Haible wrote: Still: Does ReleaseSRWLockExclusive notify other threads? Of course? How else would a lock work, it must release other waiters? It might not be a fair lock though, which is not a problem for this situation, which does not require fair locking.

Re: [PATCH v2] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

2024-05-30 Thread Bruno Haible via Cygwin
Takashi Yano wrote in cygwin-patches: > int > pthread::once (pthread_once_t *once_control, void (*init_routine) (void)) > { > - // already done ? > - if (once_control->state) > + /* Sign bit of once_control->state is used as done flag */ > + if (once_control->state & INT_MIN) > return

Re: [PATCH v2] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

2024-05-30 Thread Bruno Haible via Cygwin
Noel Grandin wrote: > > SRW locks are spin-locks. Since they are only pointer-sized, > > ReleaseSRWLockExclusive cannot notify other threads — unlike > > CRITICAL_SECTION. > > Therefore, AcquireSRWLockExclusive must busy-loop when the lock is already > > held. > > > > No, they only spin

Re: [PATCH v2] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

2024-05-30 Thread Noel Grandin via Cygwin
On 5/30/2024 10:47 AM, Bruno Haible wrote: SRW locks are spin-locks. Since they are only pointer-sized, ReleaseSRWLockExclusive cannot notify other threads — unlike CRITICAL_SECTION. Therefore, AcquireSRWLockExclusive must busy-loop when the lock is already held. No, they only spin

Re: [PATCH v2] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

2024-05-30 Thread Bruno Haible via Cygwin
Noel Grandin wrote in cygwin-patches: > Pardon my ignorance, but why not rather use the Windows SRWLock functionality? > https://learn.microsoft.com/en-us/windows/win32/sync/slim-reader-writer--srw--locks > > SRW locks are very fast, only require a single pointer-sized storage area, > can be

[PATCH v2] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

2024-05-29 Thread Takashi Yano
To avoid race issues, pthread::once() uses pthread_mutex. This caused the handle leak which was fixed by the commit 2c5433e5da82. However, this fix introduced another race issue, i.e., the mutex may be used after it is destroyed. This patch fixes the issue. Special thanks to Bruno Haible for