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

From: Jeff King <peff@peff.net>
Date: Dec 29, 2024, 16:57 UTC
Message-ID: <20241229165715.GA3158370@coredump.intra.peff.net>
In-Reply-To: <965bd864-3ac0-454f-b7bb-f8e0214e9969@web.de>

On Sun, Dec 29, 2024 at 01:02:13PM +0100, René Scharfe wrote:

Show 6 quoted lines

> 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:
> [...]
> 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?

Yep, that matches what I'd found so far.

One of the reasons I hadn't sent anything is that I was waffling between two approaches:

  - implement barriers everywhere and just use them. More work, but we'd
    have the tool if we wanted to use it later, and all builds behave
    the same.

  - make a "maybe_barrier" interface that might be a noop, and let most
    platforms compile without them. They are not needed for correct
    operation in most cases, but only to work around a sanitizer problem.
    And it is not even a problem that comes up frequently; it is a race
    that we occasionally see in CI. So enabling it only for our
    linux-leaks CI job would be enough to dull the pain.

    And there is no risk of any portability or run-time issues, because
    the code is a noop for most builds.

Show 5 quoted lines

> 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.

Yeah, I found a similar one. I think it's an undergrad OS assignment to implement barriers using semaphores, but probably building on a mutex and cond is less horrible. ;)

-Peff