summaryrefslogtreecommitdiffstats
path: root/ci
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2024-12-30 05:28:30 +0100
committerJunio C Hamano <gitster@pobox.com>2024-12-30 15:18:57 +0100
commit7d0037b59ae0d22a2718c28d8e70e3ef3f3f991e (patch)
tree1cee2d6586d9bf95471ca34b0b44bd195ee21ea2 /ci
parentRevert "index-pack: spawn threads atomically" (diff)
downloadgit-7d0037b59ae0d22a2718c28d8e70e3ef3f3f991e.tar.xz
git-7d0037b59ae0d22a2718c28d8e70e3ef3f3f991e.zip
thread-utils: introduce optional barrier type
One thread primitive we don't yet support is a barrier: it waits for all threads to reach a synchronization point before letting any of them continue. This would be useful for avoiding the LSan race we see in index-pack (and other places) by having all threads complete their initialization before any of them start to do real work. POSIX introduced a pthread_barrier_t in 2004, which does what we want. But if we want to rely on it: 1. Our Windows pthread emulation would need a new set of wrapper functions. There's a Synchronization Barrier primitive there, which was introduced in Windows 8 (which is old enough for us to depend on). 2. macOS (and possibly other systems) has pthreads but not pthread_barrier_t. So there we'd have to implement our own barrier based on the mutex and cond primitives. Those are do-able, but since we only care about avoiding races in our LSan builds, there's an easier way: make it a noop on systems without a native pthread barrier. This patch introduces a "maybe_thread_barrier" API. The clunky name (rather than just using pthread_barrier directly) should hopefully clue people in that on some systems it will do nothing. It's wired to a Makefile knob which has to be triggered manually, and we enable it for the linux-leaks CI jobs (since we know we'll have it there). There are some other possible options: - we could turn it on all the time for Linux systems based on uname. But we really only care about it for LSan builds, and there is no need to add extra code to regular builds. - we could turn it on only for LSan builds. But that would break builds on non-Linux platforms (like macOS) that otherwise should support sanitizers. - we could trigger only on the combination of Linux and LSan together. This isn't too hard to do, but the uname check isn't completely accurate. It is really about what your libc supports, and non-glibc systems might not have it (though at least musl seems to). So we'd risk breaking builds on those systems, which would need to add a new knob. Though the upside would be that running local "make SANITIZE=leak test" would be protected automatically. And of course none of this protects LSan runs from races on systems without pthread barriers. It's probably OK in practice to protect only our CI jobs, though. The race is rare-ish and most leak-checking happens through CI. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'ci')
-rwxr-xr-xci/lib.sh1
1 files changed, 1 insertions, 0 deletions
diff --git a/ci/lib.sh b/ci/lib.sh
index 8885ee3c3f..6a1267fbcb 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -385,6 +385,7 @@ linux-musl)
;;
linux-leaks|linux-reftable-leaks)
export SANITIZE=leak
+ export THREAD_BARRIER_PTHREAD=1
;;
linux-asan-ubsan)
export SANITIZE=address,undefined