On 04/07/2016 17:38, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lur...@redhat.com>
>
> 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>,
> msg=msg@entry=0x555555c38c30 <__func__.14622> "qemu_mutex_destroy")
> at /home/drjones/code/qemu/util/qemu-thread-posix.c:39
> #3 0x0000555555b0be20 in qemu_mutex_destroy
> (mutex=mutex@entry=0x5555566aa0e0) 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,
> buf=buf@entry=0x7fffc343fd98 "0q", len=len@entry=2) 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 <marcandre.lur...@redhat.com>
> Reported-by: Andrew Jones <drjo...@redhat.com>
> ---
> 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
