On Wed, Feb 26, 2025 at 09:50:15AM +0100, Thomas Huth wrote: > When compiling QEMU with --enable-trace-backends=simple , the > iotest 233 is currently hanging. This happens because qemu-nbd > calls trace_init_backends() first - which causes simpletrace to > install its writer thread and the atexit() handler - before > calling fork(). But the simpletrace writer thread is then only > available in the parent process, not in the child process anymore. > Thus when the child process exits, its atexit handler waits forever > on the trace_empty_cond condition to be set by the non-existing > writer thread, so the process never finishes. > > Fix it by installing a pthread_atfork() handler, too, which > makes sure that the trace_writeout_enabled variable gets set > to false again in the child process, so we can use it in the > atexit() handler to check whether we still need to wait on the > writer thread or not. > > Signed-off-by: Thomas Huth <th...@redhat.com> > --- > trace/simple.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/trace/simple.c b/trace/simple.c > index c0aba00cb7f..269bbda69f1 100644 > --- a/trace/simple.c > +++ b/trace/simple.c > @@ -380,8 +380,22 @@ void st_print_trace_file_status(void) > > void st_flush_trace_buffer(void) > { > - flush_trace_file(true); > + flush_trace_file(trace_writeout_enabled); > +} > + > +#ifndef _WIN32 > +static void trace_thread_atfork(void) > +{ > + /* > + * If we fork, the writer thread does not exist in the child, so > + * make sure to allow st_flush_trace_buffer() to clean up correctly. > + */ > + g_mutex_lock(&trace_lock);
And are we sure trace_lock was previously initialized in memory visible to the thread that is calling the after-fork handler here? POSIX admits that pthread_atfork() is generally too hard to use successfully (there are just too many corner cases), and instead recommends that the only portable thing a multi-threaded app can do after forking is limit itself to async-signal-safe functions until it execs a new program. The other common reason for forking is for daemonization; there, the advice is to be completely single-threaded until after the fork() has set up the right environment for the daemon, at which point the child can then finally go multi-threaded. And with either of those solutions (Using fork() to spawn a child process? do nothing unsafe after fork except for what it takes to reach exec. Using fork() to daemonize? do nothing unsafe before fork) eliminates the need for using pthread_atfork() in the first place. At any rate, per your plea elsewhere in the thread, I'll take a look at deferring the logging init until after qemu-nbd has daemonized. -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org