public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
From: Bruno Haible <bruno@clisp.org>
To: cygwin-patches@cygwin.com, Takashi Yano <takashi.yano@nifty.ne.jp>
Subject: Re: [PATCH] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82
Date: Wed, 29 May 2024 13:17:47 +0200	[thread overview]
Message-ID: <3767625.UuxTggG6dJ@nimes> (raw)
In-Reply-To: <20240529103020.53514-1-takashi.yano@nifty.ne.jp>

Takashi Yano wrote:
> 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. With this patch, do not use pthread_mutex in
> pthread::once() to avoid both issues. Instead, InterlockedExchage()
> is used.

This patch is bogus as well, because it allows one thread to return
from a pthread_once call while the other thread is currently
executing the init_routine and not yet done with it.

> +  if (!InterlockedExchange (&once_control->state, 1))
> +    init_routine ();
>    return 0;
>  }

There is no code after the init_routine () call here. This means
that other threads are not notified when the init_routine () call
is complete. Therefore this implementation *cannot* be correct.

See: Assume thread1 and thread2 call pthread_once on the same
once_control.

            thread1                      thread2
            -------                      -------

         enters pthread_once       enters pthread_once

         sets state to 1

                                   sees that state == 1

                                   returns from pthread_once

                                   executes code that assumes
                                   init_routine has completed

         starts executing
         init_routine

         finished executing
         init_routine

         returns from pthread_once

Bruno




  reply	other threads:[~2024-05-29 11:17 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-29 10:30 Takashi Yano
2024-05-29 11:17 ` Bruno Haible [this message]
2024-05-30  5:04   ` Takashi Yano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3767625.UuxTggG6dJ@nimes \
    --to=bruno@clisp.org \
    --cc=cygwin-patches@cygwin.com \
    --cc=takashi.yano@nifty.ne.jp \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).