Re: [Qemu-devel] [PATCH] char: do not use atexit cleanup handler

[Paolo Bonzini]

On 04/07/2016 17:38, address@hidden wrote:
> From: Marc-André Lureau <address@hidden>
> 
> It turns out qemu is calling exit() in various places from various
> threads without taking much care of resources state. The atexit()
> cleanup handlers cannot easily destroy resources that are in use (by
> the same thread or other).
> 
> Since c1111a24a3, TCG arm guests run into the following abort() when
> running tests, the chardev mutex is locked during the write, so
> qemu_mutex_destroy() returns an error:
> 
>  #0  0x00007fffdbb806f5 in raise () at /lib64/libc.so.6
>  #1  0x00007fffdbb822fa in abort () at /lib64/libc.so.6
>  #2  0x00005555557616fe in error_exit (err=<optimized out>, address@hidden 
> <__func__.14622> "qemu_mutex_destroy")
>      at /home/drjones/code/qemu/util/qemu-thread-posix.c:39
>  #3  0x0000555555b0be20 in qemu_mutex_destroy (address@hidden) at 
> /home/drjones/code/qemu/util/qemu-thread-posix.c:57
>  #4  0x00005555558aab00 in qemu_chr_free_common (chr=0x5555566aa0e0) at 
> /home/drjones/code/qemu/qemu-char.c:4029
>  #5  0x00005555558b05f9 in qemu_chr_delete (chr=<optimized out>) at 
> /home/drjones/code/qemu/qemu-char.c:4038
>  #6  0x00005555558b05f9 in qemu_chr_delete (chr=<optimized out>) at 
> /home/drjones/code/qemu/qemu-char.c:4044
>  #7  0x00005555558b062c in qemu_chr_cleanup () at 
> /home/drjones/code/qemu/qemu-char.c:4557
>  #8  0x00007fffdbb851e8 in __run_exit_handlers () at /lib64/libc.so.6
>  #9  0x00007fffdbb85235 in  () at /lib64/libc.so.6
>  #10 0x00005555558d1b39 in testdev_write (testdev=0x5555566aa0a0) at 
> /home/drjones/code/qemu/backends/testdev.c:71
>  #11 0x00005555558d1b39 in testdev_write (chr=<optimized out>, 
> buf=0x7fffc343fd9a "", len=0) at /home/drjones/code/qemu/backends/testdev.c:95
>  #12 0x00005555558adced in qemu_chr_fe_write (s=0x5555566aa0e0, 
> address@hidden "0q", address@hidden) at 
> /home/drjones/code/qemu/qemu-char.c:282
> 
> Instead of using a atexit() handler, only run the chardev cleanup as
> initially proposed at the end of main(), where there are less chances
> (hic) of conflicts or other races.
> 
> Signed-off-by: Marc-André Lureau <address@hidden>
> Reported-by: Andrew Jones <address@hidden>
> ---
>  include/sysemu/char.h | 7 +++++++
>  qemu-char.c           | 4 +---
>  vl.c                  | 1 +
>  3 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index 57df10a..0ea9eac 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -151,6 +151,13 @@ CharDriverState *qemu_chr_new(const char *label, const 
> char *filename,
>   */
>  void qemu_chr_disconnect(CharDriverState *chr);
>  
> +/**
> + * @qemu_chr_cleanup:
> + *
> + * Delete all chardevs (when leaving qemu)
> + */
> +void qemu_chr_cleanup(void);
> +
>  /**
>   * @qemu_chr_new_noreplay:
>   *
> diff --git a/qemu-char.c b/qemu-char.c
> index b73969d..a542192 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -4549,7 +4549,7 @@ void qmp_chardev_remove(const char *id, Error **errp)
>      qemu_chr_delete(chr);
>  }
>  
> -static void qemu_chr_cleanup(void)
> +void qemu_chr_cleanup(void)
>  {
>      CharDriverState *chr, *tmp;
>  
> @@ -4604,8 +4604,6 @@ static void register_types(void)
>       * is specified
>       */
>      qemu_add_machine_init_done_notifier(&muxes_realize_notify);
> -
> -    atexit(qemu_chr_cleanup);
>  }
>  
>  type_init(register_types);
> diff --git a/vl.c b/vl.c
> index 9bb7f4c..d0b9ff9 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4596,6 +4596,7 @@ int main(int argc, char **argv, char **envp)
>  #ifdef CONFIG_TPM
>      tpm_cleanup();
>  #endif
> +    qemu_chr_cleanup();
>  
>      return 0;
>  }
> 

Queued, thanks.

Paolo