[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Libjit] [PATCH] windows: fix race condition in _jit_thread_init()
From: |
Aleksey Demakov |
Subject: |
Re: [Libjit] [PATCH] windows: fix race condition in _jit_thread_init() |
Date: |
Tue, 21 Jan 2014 21:14:21 +0700 |
Hi Ben,
Thanks for the patch. I believe there is no need for
InterlockedCompareExchange in the case 1. It should be possible to
spin with something like this:
while (once_control != 2)
{
_ReadWriteBarrier();
}
I don't have a win32 box handy to test this, could you please give it a try?
Thanks,
Aleksey
On Tue, Jan 21, 2014 at 2:26 AM, Ben Noordhuis <address@hidden> wrote:
> On POSIX platforms, libjit uses a pthread_once_t guard for its one-time
> initialization logic.
>
> pthread_once() guarantees that in the contended case, one thread gets
> to run the initialization callback while the other threads are blocked
> until the callback returns.
>
> The Windows initialization code currently has no such guarantees.
> It guards against invoking init_win32_thread() twice but it does
> nothing to stop other threads from returning prematurely.
>
> That's bad because it means the global mutex and the thread-local
> storage may not have been initialized when _jit_thread_init() returns.
>
> This commit addresses that with an ersatz spinlock. Spinning is not
> very nice but the alternatives are arguably worse: INIT_ONCE is not
> available on Windows XP while CreateEvent() can fail in low memory
> situations.
>
> Caveat emptor: I have not actually tested the patch. I'm reasonably
> confident that it works but it's probably best to view it as a bug
> report with suggested fix attached. ;-)
>
> Signed-off-by: Ben Noordhuis <address@hidden>
> ---
> ChangeLog | 5 +++++
> jit/jit-thread.c | 12 ++++++++++--
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index a0b6bbb..ff6ec4a 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2014-01-20 Ben Noordhuis <address@hidden>
> +
> + * jit/jit-thread.c: Fix Windows-only race condition in
> + _jit_thread_init().
> +
> 2013-10-17 Aleksey Demakov <address@hidden>
>
> * README, doc/libjit.texi: Update info on obtaining libjit sources.
> diff --git a/jit/jit-thread.c b/jit/jit-thread.c
> index da3aae6..31ea8ba 100644
> --- a/jit/jit-thread.c
> +++ b/jit/jit-thread.c
> @@ -103,9 +103,17 @@ void _jit_thread_init(void)
> pthread_once(&once_control, init_pthread);
> #elif defined(JIT_THREADS_WIN32)
> static LONG volatile once_control = 0;
> - if(!InterlockedExchange((PLONG)&once_control, 1))
> + switch(InterlockedCompareExchange(&once_control, 1, 0))
> {
> - init_win32_thread();
> + case 0:
> + init_win32_thread();
> + InterlockedExchange(&once_control, 2);
> + break;
> + case 1:
> + while(InterlockedCompareExchange(&once_control, 2, 2)
> != 2)
> + {
> + }
> + break;
> }
> #endif
> }
> --
> 1.8.4.2
>
>