Handle terminating signals.

Hi,

This patch makes qemu handle signals better. For normal signals (SIGINT, SIGTERM & friends) it just sets the request_shutdown flag, making the main_loop exit and qemu taking the usual exit route, with atexit handlers being called and so on, instead of qemu just being killed by the signal.

For fatal signals such as SIGSEGV, where the exit handler can't return, it allows to register clean up handlers which will be called instead of the atexit handlers. Those handlers must be save to run in signal context.

please apply,
Gerd

http://kraxel.fedorapeople.org/xenner/

From: Gerd Hoffmann <[email protected]>
Date: Tue, 5 Aug 2008 17:53:36 +0200
Subject: [PATCH] Handle terminating signals.

This patch makes qemu handle signals better. For normal signals (SIGINT, SIGTERM & friends) it just sets the request_shutdown flag, making the main_loop exit and qemu taking the usual exit route, with atexit handlers being called and so on, instead of qemu just being killed by the signal.

For fatal signals such as SIGSEGV, where the exit handler can't return, it allows to register clean up handlers which will be called instead of the atexit handlers. Those handlers must be save to run in signal context.

---
 Makefile.target |    2 +-
 curses.c        |    2 -
 sdl.c           |    9 +----
 termsig.c       |  104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 termsig.h       |   15 ++++++++
 vl.c            |    6 +++
 6 files changed, 127 insertions(+), 11 deletions(-)
 create mode 100644 termsig.c
 create mode 100644 termsig.h

index 42162c3..ae05af0 100644
@@ -472,7 +472,7 @@ endif #CONFIG_DARWIN_USER
 # System emulator target
 ifndef CONFIG_USER_ONLY
 
-OBJS=vl.o osdep.o monitor.o pci.o loader.o isa_mmio.o machine.o net-checksum.o
+OBJS=vl.o osdep.o monitor.o pci.o loader.o isa_mmio.o machine.o net-checksum.o termsig.o
 ifdef CONFIG_WIN32
 OBJS+=block-raw-win32.o
 else

index 87aa9b3..03580fb 100644
@@ -346,8 +346,6 @@ void curses_display_init(DisplayState *ds, int full_screen)
     atexit(curses_atexit);
 
 #ifndef _WIN32
-    signal(SIGINT, SIG_DFL);
-    signal(SIGQUIT, SIG_DFL);
 #if defined(SIGWINCH) && defined(KEY_RESIZE)
     /* some curses implementations provide a handler, but we
      * want to be sure this is handled regardless of the library */

index 0edc4a0..84a9d6d 100644
@@ -476,10 +476,8 @@ static void sdl_refresh(DisplayState *ds)
                 sdl_process_key(&ev->key);
             break;
         case SDL_QUIT:
-            if (!no_quit) {
+            if (!no_quit)
                 qemu_system_shutdown_request();
-                vm_start();	/* In case we're paused */
-            }
             break;
         case SDL_MOUSEMOTION:
             if (gui_grab || kbd_mouse_is_absolute() ||

@@ -636,11 +634,6 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
         fprintf(stderr, "Could not initialize SDL - exiting\n");
         exit(1);
     }
-#ifndef _WIN32
-    /* NOTE: we still want Ctrl-C to work, so we undo the SDL redirections */
-    signal(SIGINT, SIG_DFL);
-    signal(SIGQUIT, SIG_DFL);
-#endif
 
     ds->dpy_update = sdl_update;
     ds->dpy_resize = sdl_resize;

new file mode 100644
index 0000000..1cc1cdb
@@ -0,0 +1,15 @@
+/*
+ * Register cleanup handler.
+ *
+ * These handlers are *only* called for emergeny exits, in case we
+ * can't use the normal exit() and thus atexit() handlers don't work.
+ *
+ * These handlers are called directly from the signal handler and
+ * thus are limited to functions which can safely be called from
+ * signal context (see "man 7 signal").
+ *
+ */
+void fatalsig_register_handler(void (*func)(void));
+
+/* setup signal handlers, called from main() */
+void termsig_setup(void);

index e929370..56de905 100644
@@ -37,6 +37,7 @@
 #include "qemu-char.h"
 #include "block.h"
 #include "audio/audio.h"
+#include "termsig.h"
 
 #include <unistd.h>
 #include <fcntl.h>

@@ -7585,6 +7586,8 @@ static int main_loop(void)
                 timeout = 0;
             }
         } else {
+            if (shutdown_requested)
+                break;
             timeout = 10;
         }
 #ifdef CONFIG_PROFILER

@@ -9031,6 +9034,9 @@ int main(int argc, char **argv)
 #endif
     }
 
+    /* must be after terminal init, SDL changes signal handlers */
+    termsig_setup();
+
     /* Maintain compatibility with multiple stdio monitors */
 
     has_monitor = 0;

1.5.5.1

Gerd Hoffmann writes ("[Qemu-devel] [PATCH] Handle terminating signals."):

Ok, we could take the SIGSEGV cleanup route, then call abort(). That should come close enougth. Does that sound ok?

cheers,
Gerd

http://kraxel.fedorapeople.org/xenner/

Gerd Hoffmann writes ("Re: [Qemu-devel] [PATCH] Handle terminating signals."):

No, because the program should not attempt to catch SEGV either.

Ian.

I agree.
The idea of trying a clean exit when a termination signal is received is good, but do we really need the complicated fatalsig_call_handlers and fatalsig_register_handler system?

Why not? Can you change your attitude to say "no" without giving reasons please?

cheers,
Gerd

http://kraxel.fedorapeople.org/xenner/

In message: <[email protected]>
Gerd Hoffmann <[email protected]> writes:
: Ian Jackson wrote:
: > Gerd Hoffmann writes ("Re: [Qemu-devel] [PATCH] Handle terminating signals."):
: >> Ian Jackson wrote:
: >>> SIGQUIT should not be in this list. QUIT does not mean please : >>> terminate'. It's a signal used for debugging purposes and usually : >>> means please pretend this program took a SEGV'. The default action : >>> is to die and dump core, and this should be preserved.
: >> Ok, we could take the SIGSEGV cleanup route, then call abort(). That : >> should come close enougth. Does that sound ok?
: >
: > No, because the program should not attempt to catch SEGV either. :
: Why not? Can you change your attitude to say "no" without giving
: reasons please?

The only portable thing one can do when catching SEGV is terminate the program. Otherwise, when the signal handler returns, SEGV happens again...

Warner

Returning from the signal handler isn't going to work, sure. The only thing I want do is cleaning up before exiting.

Most apps never ever have to care about that. Sometimes there are good reasons to attempt a cleanup even for a SEGV though. The X-Server for example attempts to put the gfx card into a sane state then.

cheers,
Gerd

http://kraxel.fedorapeople.org/xenner/

Note my previous suggestion of pipe() + fork() gives you the desired cleanup behaviour without having to deal with any of this silliness. IMHO, the real problem here is that Xen's domain-0 interface isn't based on file descriptors to begin with. What modern Linux interface that requires a clean-up action isn't, afterall?

Regards,

Anthony Liguori

The X server often fails to do this cleanup. This is why they're moving mode setting into the kernel so it can be *reliably* reset. Relying on a SEGV handler for cleanup is just doomed to fail

Daniel

|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Gerd Hoffmann writes ("Re: [Qemu-devel] [PATCH] Handle terminating signals."):

I guess this is another one of those pieces of `obvious' wisdom which no-one previously bothered writing down. I tried to find a clear online reference which explains why but I couldn't find one, so I'll try to explain it here:

If your program gets a SEGV, that means its memory may be corrupted. Because the signal is asynchronous and may happen at any point, the values of variables and data structures may be arbitrary and of course inconsistent (since perhaps the program was halfway through modifying a data structure). The contents of the stack cannot be relied on. Even the stack pointer may be corrupted (that might be the cause of the crash).

Technically, _any_ attempt to resume the mainline program execution (whether by returning or longjmping out of the SEGV handler, or by running the same code inside the signal handler) has undefined behaviour. That includes _any attempt to access a global variable_ (other than a variable which is specially marked and protected, and of course where the signal blocking is used appropriately - which is not and cannot be the case here).

This means that attempts to recover' or clean up' when you get SEGV are at least as likely to make things worse as they are to make things better. Furthermore bugs tend to happen in complex corner cases, which is precisely the kind of situation where trying to continue or tidy up after a SEGV is likely to do harm. Even if the recovery code doesn't actually worsen the data corruption and loss, it will make the situation more confusing by making additional changes to the system state.

That means it's much harder for a system administrator to recover or repair, and also much harder for the operating system or application's crash recovery code to cope with. Post-crash data recovery is a well-developed field; there are well-understood strategies adopted by humans and computers under various circumstances. These are likely to be defeated by attempts to `clean up' between the detection of a fatal corruption and actual death.

Both as a system administrators and as a programmers I curse programs which try to trap fatal bugs like this. It makes it harder to figure out why the system is failing; it makes it harder to restore the system to a working and correct state; it makes it harder to avoid data corruption; and in the worst it can also lead to a cavalier attitude towards bugs.

Thus the best thing to do - usually the only sane thing to do - with the coredumping signals (SEGV, BUS, ABRT, QUIT, as applicable and available) is to leave them unblocked and set to SIG_DFL.

(SIGQUIT is a different argument because it's only ever sent explicitly. But its purpose is to aid debugging and get an instantaneous coredump, which would be defeated by catching it.)

Ian.