[Bug 47054] Make mission-control quit gracefully

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=47054

--- Comment #2 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2012-03-13 07:55:55 PDT ---
(In reply to comment #1)
> Simon is claiming that "using POSIX signals like that isn't safe
> unfortunately". Over to you, Simon.

There's a relatively short list of things you can safely/portably do in a POSIX
signal()/sigaction() handler. g_main_loop_quit() isn't one of them; neither is
writing to a global variable (!) except with special precautions. A reasonable
number of syscalls (but not all) are safe, though. The magic words to look for
are "async-signal safe".

The signal(7) man page has the complete list of safe syscalls. In particular,
write() and close() are safe.

The traditional way to handle a signal safely is to have a pipe-to-self, which
you set up before entering the main loop. When you receive a signal, you write
to the pipe (1 byte is enough). bus/main.c in dbus does this correctly.

All the signal handling should be #ifdef G_OS_UNIX, to avoid having to try to
be portable at the same time as restricting yourself to async-signal-safe
syscalls. signal()/sigaction() aren't portable beyond Unix anyway.

In GLib 2.32 we'll be able to use g_unix_signal_add() which is meant to do all
this for you... but 2.32 isn't out yet, and I just looked at the implementation
and found that it assumes single-byte writes to a volatile gchar[] are atomic,
which I don't think is actually guaranteed.

I'd personally be inclined to use SIGINT (Ctrl+C) as the "graceful quit"
signal, rather than SIGUSR1.

> + signal (SIGUSR1, sigusr1_cb);

The man page for signal() basically says "don't use this function": sigaction()
is preferred (it has the same semantics on all platforms, whereas signal()
varies).

> +static void
> +sigusr1_cb (int sig)
> +{
> + mcd_service_stop (mcd);
> }

Optional changes:

After you've changed this to be at the "read" end of the pipe-to-self, it might
be better to have it raise the abort signal on the McdService
(mcd_mission_abort) so that things get unreffed cleanly? That would hopefully
mean we can eventually valgrind MC without having to use the special debug
build and signal it over D-Bus.

>From a valgrind point of view, it might also be good to do the actual main loop
quit in a low-priority idle after the abort signal has happened, so that MC has
a chance to clean up internal objects that were reffed by higher-priority
idles. (The special debug build currently uses a 5 second timeout instead of an
idle, but that seems as though it shouldn't be needed, ideally.)

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.
You are the assignee for the bug.