Re: racy leak sanitizer builds, was Re: [PATCH 0/9] commit-reach: -Wsign-compare follow-ups

From: René Scharfe <l.s.r@web.de>
Date: Dec 29, 2024, 12:02 UTC
Message-ID: <965bd864-3ac0-454f-b7bb-f8e0214e9969@web.de>
In-Reply-To: <20241228192307.GC815586@coredump.intra.peff.net>

Am 28.12.2024 um 20:23 schrieb Jeff King:

Show 17 quoted lines

> On Sat, Dec 28, 2024 at 02:05:41PM -0500, Jeff King wrote:
>
>> So I suspect the race is actually trickier, and that the "weird state"
>> is not something that happens just while pthread_create() is being
>> called, but is actually running _in the thread itself_. So even though
>> pthread_create() has returned for each thread, they are still setting
>> themselves up before running.
>> [...]
>> So a full fix would actually require synchronization where we spawn each
>> thread, then wait for all of them to hit the barrier to declare
>> themselves ready, and then let them all start running. There is a
>> pthread_barrier type that would help with this, but we've never used it
>> before (so we'd probably need to at least provide a Windows compat
>> layer).
>
> So here's a fix that seems to work, but doesn't address the portability
> issues.

Windows has Synchronization Barriers. Adding the following lines to compat/win32/pthread.h at least lets your example compile and run:

#include <synchapi.h> #define pthread_barrier_t SYNCHRONIZATION_BARRIER #define PTHREAD_BARRIER_SERIAL_THREAD TRUE #define pthread_barrier_init(b, a, c) return_0(InitializeSynchronizationBarrier((b), (c), -1)) #define pthread_barrier_destroy(b) return_0(DeleteSynchronizationBarrier(b)) #define pthread_barrier_wait(b) EnterSynchronizationBarrier((b), 0)

Catching a non-NULL second argument to pthread_barrier_init() would be a good idea in a production version. Error handling would be a good idea in general, but callers would then actually have to check those errors.

Synchronization Barriers were added with Windows 8 and Windows Server 2012, Git for Windows requires higher versions, so this native mechanism should be usable. Relevant links:

https://learn.microsoft.com/en-us/windows/win32/sync/synchronization-barriers https://github.com/git-for-windows/git/wiki/FAQ

However, macOS doesn't have pthread barriers. Here's an implementation that had to be fixed to satisfy Coverity, so it might be good now?

https://github.com/libusb/hidapi/blob/master/mac/hid.c

Perhaps that implementation could be used for Windows as well? All functions it uses are provided by compat/win32/pthread.h; not sure if they are sufficiently fleshed out, though.

René