Re: [Qemu-devel] [PATCH v2] trace/simple: fix hang in child after fork(2)

[Paolo Bonzini]

On 13/07/2018 20:42, Stefan Hajnoczi wrote:
> +#ifndef _WIN32
> +static void stop_writeout_thread(void)
> +{
> +    g_mutex_lock(&trace_lock);
> +    trace_writeout_running = false;
> +    g_cond_signal(&trace_available_cond);
> +    g_mutex_unlock(&trace_lock);
> +
> +    g_thread_join(trace_writeout_thread);
> +    trace_writeout_thread = NULL;
> +}

After stop_writeout_thread returns, another could start a write to the
shared data structure---and the write would never finish, because the
thread disappears after fork(2) returns.  This would leave the mutex
locked, causing a deadlock soon after the fork.  So you need to lock
trace_lock again here, and unlock it in restart_writeout_thread.

Apart from this, it looks good!

Thanks,

Paolo

> +static void restart_writeout_thread(void)
> +{
> +    trace_writeout_running = true;
> +    trace_writeout_thread = trace_thread_create(writeout_thread);
> +    if (!trace_writeout_thread) {
> +        warn_report("unable to initialize simple trace backend");
> +    }
> +}
> +#endif /* !_WIN32 */
> +
>  bool st_init(void)
>  {
> -    GThread *thread;
> -
>      trace_pid = getpid();
> +    trace_writeout_running = true;
>  
> -    thread = trace_thread_create(writeout_thread);
> -    if (!thread) {
> +    trace_writeout_thread = trace_thread_create(writeout_thread);
> +    if (!trace_writeout_thread) {
>          warn_report("unable to initialize simple trace backend");
>          return false;
>      }
>  
> +#ifndef _WIN32
> +    /* Terminate writeout thread across fork and restart it in parent and
> +     * child afterwards.
> +     */
> +    pthread_atfork(stop_writeout_thread,
> +                   restart_writeout_thread,
> +                   restart_writeout_thread);
> +#endif
> +
>      atexit(st_flush_trace_buffer);
>      return true;
>  }
>