Skip to content

Commit

Permalink
char: do not use atexit cleanup handler
Browse files Browse the repository at this point in the history
RH-Author: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-id: <20160704205732.27022-6-marcandre.lureau@redhat.com>
Patchwork-id: 70942
O-Subject: [RHEV-7.3 qemu-kvm-rhev PATCH v2 5/5] char: do not use atexit cleanup handler
Bugzilla: 1347077
RH-Acked-by: Xiao Wang <jasowang@redhat.com>
RH-Acked-by: Laurent Vivier <lvivier@redhat.com>
RH-Acked-by: Victor Kaplansky <vkaplans@redhat.com>
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>

From: Marc-André Lureau <marcandre.lureau@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 c1111a2, 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
 open-power-host-os#4  0x00005555558aab00 in qemu_chr_free_common (chr=0x5555566aa0e0) at /home/drjones/code/qemu/qemu-char.c:4029
 open-power-host-os#5  0x00005555558b05f9 in qemu_chr_delete (chr=<optimized out>) at /home/drjones/code/qemu/qemu-char.c:4038
 open-power-host-os#6  0x00005555558b05f9 in qemu_chr_delete (chr=<optimized out>) at /home/drjones/code/qemu/qemu-char.c:4044
 open-power-host-os#7  0x00005555558b062c in qemu_chr_cleanup () at /home/drjones/code/qemu/qemu-char.c:4557
 open-power-host-os#8  0x00007fffdbb851e8 in __run_exit_handlers () at /lib64/libc.so.6
 open-power-host-os#9  0x00007fffdbb85235 in  () at /lib64/libc.so.6
 open-power-host-os#10 0x00005555558d1b39 in testdev_write (testdev=0x5555566aa0a0) at /home/drjones/code/qemu/backends/testdev.c:71
 open-power-host-os#11 0x00005555558d1b39 in testdev_write (chr=<optimized out>, buf=0x7fffc343fd9a "", len=0) at /home/drjones/code/qemu/backends/testdev.c:95
 open-power-host-os#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.lureau@redhat.com>
Reported-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

(cherry picked from commit dd9250d5d99a2671584f65b7d0745355bbbe353f)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
  • Loading branch information
elmarco authored and cuinutanix committed Mar 9, 2017
1 parent 9fa60de commit b384aa5
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 3 deletions.
7 changes: 7 additions & 0 deletions include/sysemu/char.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
*
Expand Down
4 changes: 1 addition & 3 deletions qemu-char.c
Original file line number Diff line number Diff line change
Expand Up @@ -4566,7 +4566,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;

Expand Down Expand Up @@ -4621,8 +4621,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);
1 change: 1 addition & 0 deletions vl.c
Original file line number Diff line number Diff line change
Expand Up @@ -4673,6 +4673,7 @@ int main(int argc, char **argv, char **envp)
#ifdef CONFIG_TPM
tpm_cleanup();
#endif
qemu_chr_cleanup();

return 0;
}

0 comments on commit b384aa5

Please sign in to comment.