# HG changeset patch # User Mark Doliner # Date 1257374838 0 # Node ID 5063f721de6d7e7573c6e6b84899b83d45fb0332 # Parent bb923bcae9de3e6fddb332ab514b5b0ca0d05408 We really shouldn't be doing a whole lot in our signal handler. The signal handler can get called in the middle of a malloc, for example, and if the signal handler tries to allocate memory then the program can deadlock. This change works around that problem by having the signal handler write to a pipe. Our main program watches the pipe for incoming data and performs the appropriate action. This patch is from Shaun Lindsay at Meebo. Fixes #10324 diff -r bb923bcae9de -r 5063f721de6d pidgin/gtkmain.c --- a/pidgin/gtkmain.c Wed Nov 04 22:16:51 2009 +0000 +++ b/pidgin/gtkmain.c Wed Nov 04 22:47:18 2009 +0000 @@ -143,6 +143,10 @@ } #ifdef HAVE_SIGNAL_H +static char *segfault_message; + +static int signal_sockets[2]; + static void sighandler(int sig); /* @@ -168,31 +172,60 @@ } } -char *segfault_message; +static void sighandler(int sig) +{ + ssize_t written; + + /* + * We won't do any of the heavy lifting for the signal handling here + * because we have no idea what was interrupted. Previously this signal + * handler could result in some calls to malloc/free, which can cause + * deadlock in libc when the signal handler was interrupting a previous + * malloc or free. So instead we'll do an ugly hack where we write the + * signal number to one end of a socket pair. The other half of the + * socket pair is watched by our main loop. When the main loop sees new + * data on the socket it reads in the signal and performs the appropriate + * action without fear of interrupting stuff. + */ + if (sig == SIGSEGV) { + fprintf(stderr, "%s", segfault_message); + abort(); + return; + } -/* - * This signal handler shouldn't be touching this much stuff. - * It should just set a flag and return, and something else in - * Pidgin should monitor the flag to see if something needs to - * be done. Because the signal handler interrupts the program, - * it could be called in the middle of adding a new connection - * to the list of connections, and then if we try to disconnect - * all connections it could lead to a crash because the linked - * list of connections could be in a weird state. But, well, - * this signal handler probably isn't called very often, so it's - * not a big deal. - */ -static void -sighandler(int sig) + written = write(signal_sockets[0], &sig, sizeof(int)); + if (written < 0 || written != sizeof(int)) { + /* This should never happen */ + purple_debug_error("sighandler", "Received signal %d but only " + "wrote %" G_GSSIZE_FORMAT " bytes out of %" + G_GSIZE_FORMAT ": %s\n", + sig, written, sizeof(int), g_strerror(errno)); + exit(1); + } +} + +static gboolean +mainloop_sighandler(GIOChannel *source, GIOCondition cond, gpointer data) { + GIOStatus stat; + int sig; + gsize bytes_read; + GError *error = NULL; + + /* read the signal number off of the io channel */ + stat = g_io_channel_read_chars(source, (gchar *)&sig, sizeof(int), + &bytes_read, &error); + if (stat != G_IO_STATUS_NORMAL) { + purple_debug_error("sighandler", "Signal callback failed to read " + "from signal socket: %s", error->message); + purple_core_quit(); + return FALSE; + } + switch (sig) { case SIGHUP: purple_debug_warning("sighandler", "Caught signal %d\n", sig); break; - case SIGSEGV: - fprintf(stderr, "%s", segfault_message); - abort(); - break; #if defined(USE_GSTREAMER) && !defined(GST_CAN_DISABLE_FORKING) /* By default, gstreamer forks when you initialize it, and waitpids for the * child. But if libpurple reaps the child rather than leaving it to @@ -219,6 +252,8 @@ purple_debug_warning("sighandler", "Caught signal %d\n", sig); purple_core_quit(); } + + return TRUE; } #endif @@ -502,6 +537,8 @@ sigset_t sigset; RETSIGTYPE (*prev_sig_disp)(int); char errmsg[BUFSIZ]; + GIOChannel *signal_channel; + GIOStatus signal_status; #ifndef DEBUG char *segfault_message_tmp; GError *error = NULL; @@ -592,6 +629,29 @@ ); #endif + /* + * Create a socket pair for receiving unix signals from a signal + * handler. + */ + if (socketpair(AF_UNIX, SOCK_STREAM, 0, signal_sockets) < 0) { + perror("Failed to create sockets for GLib signal handling"); + exit(1); + } + signal_channel = g_io_channel_unix_new(signal_sockets[1]); + + /* + * Set the channel encoding to raw binary instead of the default of + * UTF-8, because we'll be sending integers across instead of strings. + */ + error = NULL; + signal_status = g_io_channel_set_encoding(signal_channel, NULL, &error); + if (signal_status != G_IO_STATUS_NORMAL) { + fprintf(stderr, "Failed to set the signal channel to raw " + "binary: %s", error->message); + exit(1); + } + g_io_add_watch(signal_channel, G_IO_IN, mainloop_sighandler, NULL); + /* Let's not violate any PLA's!!!! */ /* jseymour: whatever the fsck that means */ /* Robot101: for some reason things like gdm like to block *