On 01/08/2016 13:23, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lur...@redhat.com>
>
> Since aa5cb7f5e, the chardevs are being cleaned up when leaving
> qemu. However, the monitor has still references to them, which may
> lead to crashes when running atexit() and trying to send monitor
> events:
>
> #0 0x00007fffdb18f6f5 in __GI_raise (sig=sig@entry=6) at
> ../sysdeps/unix/sysv/linux/raise.c:54
> #1 0x00007fffdb1912fa in __GI_abort () at abort.c:89
> #2 0x0000555555c263e7 in error_exit (err=22, msg=0x555555d47980
> <__func__.13537> "qemu_mutex_lock") at util/qemu-thread-posix.c:39
> #3 0x0000555555c26488 in qemu_mutex_lock (mutex=0x5555567a2420) at
> util/qemu-thread-posix.c:66
> #4 0x00005555558c52db in qemu_chr_fe_write (s=0x5555567a2420,
> buf=0x55555740dc40 "{\"timestamp\": {\"seconds\": 1470041716,
> \"microseconds\": 989699}, \"event\": \"SPICE_DISCONNECTED\", \"data\":
> {\"server\": {\"port\": \"5900\", \"family\": \"ipv4\", \"host\":
> \"127.0.0.1\"}, \"client\": {\"port\": \"40272\", \"f"..., len=240) at
> qemu-char.c:280
> #5 0x0000555555787cad in monitor_flush_locked (mon=0x5555567bd9e0) at
> /home/elmarco/src/qemu/monitor.c:311
> #6 0x0000555555787e46 in monitor_puts (mon=0x5555567bd9e0,
> str=0x5555567a44ef "") at /home/elmarco/src/qemu/monitor.c:353
> #7 0x00005555557880fe in monitor_json_emitter (mon=0x5555567bd9e0,
> data=0x5555567c73a0) at /home/elmarco/src/qemu/monitor.c:401
> #8 0x00005555557882d2 in monitor_qapi_event_emit
> (event=QAPI_EVENT_SPICE_DISCONNECTED, qdict=0x5555567c73a0) at
> /home/elmarco/src/qemu/monitor.c:472
> #9 0x000055555578838f in monitor_qapi_event_queue
> (event=QAPI_EVENT_SPICE_DISCONNECTED, qdict=0x5555567c73a0,
> errp=0x7fffffffca88) at /home/elmarco/src/qemu/monitor.c:497
> #10 0x0000555555c15541 in qapi_event_send_spice_disconnected
> (server=0x5555571139d0, client=0x5555570d0db0, errp=0x5555566c0428
> <error_abort>) at qapi-event.c:1038
> #11 0x0000555555b11bc6 in channel_event (event=3, info=0x5555570d6c00) at
> ui/spice-core.c:248
> #12 0x00007fffdcc9983a in adapter_channel_event (event=3,
> info=0x5555570d6c00) at reds.c:120
> #13 0x00007fffdcc99a25 in reds_handle_channel_event (reds=0x5555567a9d60,
> event=3, info=0x5555570d6c00) at reds.c:324
> #14 0x00007fffdcc7d4c4 in main_dispatcher_self_handle_channel_event
> (self=0x5555567b28b0, event=3, info=0x5555570d6c00) at main-dispatcher.c:175
> #15 0x00007fffdcc7d5b1 in main_dispatcher_channel_event
> (self=0x5555567b28b0, event=3, info=0x5555570d6c00) at main-dispatcher.c:194
> #16 0x00007fffdcca7674 in reds_stream_push_channel_event (s=0x5555570d9910,
> event=3) at reds-stream.c:354
> #17 0x00007fffdcca749b in reds_stream_free (s=0x5555570d9910) at
> reds-stream.c:323
> #18 0x00007fffdccb5dad in snd_disconnect_channel (channel=0x5555576a89a0) at
> sound.c:229
> #19 0x00007fffdccb9e57 in snd_detach_common (worker=0x555557739720) at
> sound.c:1589
> #20 0x00007fffdccb9f0e in snd_detach_playback (sin=0x5555569fe3f8) at
> sound.c:1602
> #21 0x00007fffdcca3373 in spice_server_remove_interface (sin=0x5555569fe3f8)
> at reds.c:3387
> #22 0x00005555558ff6e2 in line_out_fini (hw=0x5555569fe370) at
> audio/spiceaudio.c:152
> #23 0x00005555558f909e in audio_atexit () at audio/audio.c:1754
> #24 0x00007fffdb1941e8 in __run_exit_handlers (status=0,
> listp=0x7fffdb5175d8 <__exit_funcs>,
> run_list_atexit=run_list_atexit@entry=true) at exit.c:82
> #25 0x00007fffdb194235 in __GI_exit (status=<optimized out>) at exit.c:104
> #26 0x00007fffdb17b738 in __libc_start_main (main=0x5555558d7874 <main>,
> argc=67, argv=0x7fffffffcf48, init=<optimized out>, fini=<optimized out>,
> rtld_fini=<optimized out>, stack_end=0x7fffffffcf38) at
> ../csu/libc-start.c:323
>
> Add a monitor_cleanup() functions to remove all the monitors before
> cleaning up the chardev. Note that we are "losing" some events that
> used to be sent during atexit().
>
> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> monitor.c | 20 ++++++++++++++++++++
> include/monitor/monitor.h | 1 +
> vl.c | 1 +
> 3 files changed, 22 insertions(+)
>
> diff --git a/monitor.c b/monitor.c
> index 5d68a5d..5c00373 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -635,6 +635,13 @@ static void monitor_data_init(Monitor *mon)
>
> static void monitor_data_destroy(Monitor *mon)
> {
> + if (mon->chr) {
> + qemu_chr_add_handlers(mon->chr, NULL, NULL, NULL, NULL);
> + }
> + if (monitor_is_qmp(mon)) {
> + json_message_parser_destroy(&mon->qmp.parser);
> + }
> + g_free(mon->rs);
> QDECREF(mon->outbuf);
> qemu_mutex_destroy(&mon->out_lock);
> }
> @@ -4196,6 +4203,19 @@ void monitor_init(CharDriverState *chr, int flags)
> qemu_mutex_unlock(&monitor_lock);
> }
>
> +void monitor_cleanup(void)
> +{
> + Monitor *mon, *next;
> +
> + qemu_mutex_lock(&monitor_lock);
> + QLIST_FOREACH_SAFE(mon, &mon_list, entry, next) {
> + QLIST_REMOVE(mon, entry);
> + monitor_data_destroy(mon);
> + g_free(mon);
> + }
> + qemu_mutex_unlock(&monitor_lock);
> +}
> +
> static void bdrv_password_cb(void *opaque, const char *password,
> void *readline_opaque)
> {
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index c5c9ea2..a714d8e 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -17,6 +17,7 @@ extern Monitor *cur_mon;
> bool monitor_cur_is_qmp(void);
>
> void monitor_init(CharDriverState *chr, int flags);
> +void monitor_cleanup(void);
>
> int monitor_suspend(Monitor *mon);
> void monitor_resume(Monitor *mon);
> diff --git a/vl.c b/vl.c
> index e7c2c62..a14c438 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4612,6 +4612,7 @@ int main(int argc, char **argv, char **envp)
>
> /* vhost-user must be cleaned up before chardevs. */
> net_cleanup();
> + monitor_cleanup();
> qemu_chr_cleanup();
>
> return 0;
>
Reviewed-by: Paolo Bonzini <pbonz...@redhat.com>
