Hi Frank, (Almost missed this because it was sent by @elastic.org with a dkim signature that says it expects a blank List-Id header, but since the list manager still adds it, it bounced because of the strict dmarc policy.)
On Tue, 2026-06-30 at 21:23 -0400, Frank Ch. Eigler wrote: > The following patch corrects the flakey test > run-backtrace-native-core.sh. Its intermittent failures have annoyed > humans and robots alike. This patch adds a posix semaphore to create > a sync point between the child thread and the main thread, to > guarantee the main thread is well out of its pthread_create() before > the child triggers the SIGABRT. Thanks, this looks like a plausible solution. Just a few nitpicks/questions below. It is mainly about the comments, the basic idea looks good. > Author: Frank Ch. Eigler <[email protected]> > Date: Tue Jun 30 19:35:56 2026 -0400 > > testsuite backtrace-child.c: Fix thread/spawn/coredump race condition. > > diff --git a/tests/backtrace-child.c b/tests/backtrace-child.c > index 8bfed478ced9..f2b313820a61 100644 > --- a/tests/backtrace-child.c > +++ b/tests/backtrace-child.c > @@ -100,6 +100,7 @@ main (int argc __attribute__ ((unused)), char **argv) > #else /* __linux__ */ > #include <sys/ptrace.h> > #include <signal.h> > +#include <semaphore.h> > > #if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 5) > #define NOINLINE_NOCLONE __attribute__ ((noinline, noclone)) > @@ -113,6 +114,11 @@ main (int argc __attribute__ ((unused)), char **argv) > > static int ptraceme, gencore; > > +/* For --gencore, the worker must not abort until the main thread has entered > + pthread_join. Otherwise the core may capture main still in pthread_create > + with no useful backtrace (seen intermittently on armhf). */ > +static sem_t gencore_go; I think the analysis that things go wrong when the main thread is still inside pthread_create is correct. I don't think it needs to enter pthread_join though (and if it would we would still have a race). It has been observed on more arches than armhf, that one might just be slow enough to trigger it more often. But it has been observed on other arches too. > /* Execution will arrive here from jmp by an artificial ptrace-spawn signal. > */ > > static NOINLINE_NOCLONE void > @@ -202,6 +208,11 @@ dummy4 (void) > static void * > start (void *arg UNUSED) > { > + if (gencore) > + { > + int err = sem_wait (&gencore_go); > + assert (err == 0); > + } OK, barrier to only continue when the main thread signals it is ready. > backtracegen (); > /* Not reached. */ > abort (); > @@ -226,7 +237,10 @@ main (int argc UNUSED, char **argv) > dummy3 (); > dummy4 (); > if (gencore) > - printf ("%ld\n", (long) getpid ()); It looks only debug output, but why remove it? > + { > + int err = sem_init (&gencore_go, 0, 0); > + assert (err == 0); > + } OK, main thread inits, before creating any other threads. > pthread_t thread; > int i = pthread_create (&thread, NULL, start, NULL); > // pthread_* functions do not set errno. > @@ -238,7 +252,13 @@ main (int argc UNUSED, char **argv) > assert (l == 0); > } > if (gencore) > - pthread_join (thread, NULL); > + { > + printf ("%ld\n", (long) getpid ()); Aha, here the printf returns, is this a better place? > + int err = sem_post (&gencore_go); > + assert (err == 0); > + pthread_join (thread, NULL); > + sem_destroy (&gencore_go); > + } OK, signal we are ready, then start waiting for the other thread, cleaning up if that succeeds. > else > raise (SIGUSR2); > return 0; I am surprised this is only necessary for the gencore case and not the signal case? Does the signal case synchronize a different way? Thanks, Mark
