Attachment 34331 Details for Bug 26303 – move sigterm handling into a pipe

[patch] move sigterm handling into a pipe

0001-bus-Move-SIGTERM-handling-into-a-pipe.patch (text/plain), 6.23 KB, created by Colin Walters on 2010-03-22 13:36:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Colin Walters

Created: 2010-03-22 13:36:10 UTC

Size: 6.23 KB

patch

obsolete

>From 4ab83e6c1769c7c911140b229829ea87d76e24d7 Mon Sep 17 00:00:00 2001
>From: Colin Walters <walters@verbum.org>
>Date: Mon, 22 Mar 2010 16:34:42 -0400
>Subject: [PATCH] [bus] Move SIGTERM handling into a pipe
>
>We really shouldn't do anything more than write() to a file
>descriptor from a signal handler for well-known reasons.
>
>This patch makes it so that we write a single character representing
>the signal to the pipe, instead of just using the pipe for
>reloads.
>---
> bus/main.c |  112 +++++++++++++++++++++++++++++++++--------------------------
> 1 files changed, 63 insertions(+), 49 deletions(-)
>
>diff --git a/bus/main.c b/bus/main.c
>index e3650f1..54c2c9b 100644
>--- a/bus/main.c
>+++ b/bus/main.c
>@@ -35,40 +35,43 @@
> 
> static BusContext *context;
> 
>-static int reload_pipe[2];
>-#define RELOAD_READ_END 0
>-#define RELOAD_WRITE_END 1
>+static int signal_pipe[2];
>+#define SIGNAL_READ_END 0
>+#define SIGNAL_WRITE_END 1
> 
>-static void close_reload_pipe (void);
>+static void close_signal_pipe (void);
> 
> static void
> signal_handler (int sig)
> {
>+  DBusString str;
> 
>   switch (sig)
>     {
>-#ifdef DBUS_BUS_ENABLE_DNOTIFY_ON_LINUX 
>-    case SIGIO: 
>+#ifdef DBUS_BUS_ENABLE_DNOTIFY_ON_LINUX
>+    case SIGIO:
>       /* explicit fall-through */
> #endif /* DBUS_BUS_ENABLE_DNOTIFY_ON_LINUX  */
> #ifdef SIGHUP
>     case SIGHUP:
>-      {
>-        DBusString str;
>-        _dbus_string_init_const (&str, "foo");
>-        if ((reload_pipe[RELOAD_WRITE_END] > 0) && 
>-            !_dbus_write_socket (reload_pipe[RELOAD_WRITE_END], &str, 0, 1))
>-          {
>-            _dbus_warn ("Unable to write to reload pipe.\n");
>-            close_reload_pipe ();
>-          }
>-      }
>+      _dbus_string_init_const (&str, "r");
>       break;
> #endif
>     case SIGTERM:
>-      _dbus_loop_quit (bus_context_get_loop (context));
>+      _dbus_string_init_const (&str, "t");
>       break;
>+    default:
>+      return;
>+    }
>+
>+  if ((signal_pipe[SIGNAL_WRITE_END] > 0) &&
>+      !_dbus_write_socket (signal_pipe[SIGNAL_WRITE_END], &str, 0, 1))
>+    {
>+      _dbus_warn ("Unable to write to reload pipe.\n");
>+       close_signal_pipe ();
>     }
>+
>+  _dbus_string_free (&str);
> }
> 
> static void
>@@ -150,38 +153,49 @@ check_two_pid_descriptors (const DBusString *pid_fd,
> }
> 
> static dbus_bool_t
>-handle_reload_watch (DBusWatch    *watch,
>-		     unsigned int  flags,
>-		     void         *data)
>+handle_signal_watch (DBusWatch    *watch,
>+                     unsigned int  flags,
>+                     void         *data)
> {
>   DBusError error;
>   DBusString str;
>+  unsigned char sigchar;
> 
>   while (!_dbus_string_init (&str))
>     _dbus_wait_for_memory ();
> 
>-  if ((reload_pipe[RELOAD_READ_END] > 0) &&
>-      _dbus_read_socket (reload_pipe[RELOAD_READ_END], &str, 1) != 1)
>+  if ((signal_pipe[SIGNAL_READ_END] > 0) &&
>+      _dbus_read_socket (signal_pipe[SIGNAL_READ_END], &str, 1) != 1)
>     {
>       _dbus_warn ("Couldn't read from reload pipe.\n");
>-      close_reload_pipe ();
>+      close_signal_pipe ();
>       return TRUE;
>     }
>-  _dbus_string_free (&str);
> 
>-  /* this can only fail if we don't understand the config file
>-   * or OOM.  Either way we should just stick with the currently
>-   * loaded config.
>-   */
>   dbus_error_init (&error);
>-  if (! bus_context_reload_config (context, &error))
>+  sigchar = _dbus_string_get_byte (&str, 0);
>+  switch (sigchar)
>     {
>-      _DBUS_ASSERT_ERROR_IS_SET (&error);
>-      _dbus_assert (dbus_error_has_name (&error, DBUS_ERROR_FAILED) ||
>-		    dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY));
>-      _dbus_warn ("Unable to reload configuration: %s\n",
>-		  error.message);
>-      dbus_error_free (&error);
>+    case 'r':
>+      /* this can only fail if we don't understand the config file
>+       * or OOM.  Either way we should just stick with the currently
>+       * loaded config.
>+       */
>+      if (! bus_context_reload_config (context, &error))
>+        {
>+          _DBUS_ASSERT_ERROR_IS_SET (&error);
>+          _dbus_assert (dbus_error_has_name (&error, DBUS_ERROR_FAILED) ||
>+                        dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY));
>+          _dbus_warn ("Unable to reload configuration: %s\n",
>+                      error.message);
>+           dbus_error_free (&error);
>+        }
>+      break;
>+    case 't':
>+      _dbus_loop_quit (bus_context_get_loop (context));
>+      break;
>+    default:
>+      break;
>     }
>   return TRUE;
> }
>@@ -195,14 +209,14 @@ reload_watch_callback (DBusWatch    *watch,
> }
> 
> static void
>-setup_reload_pipe (DBusLoop *loop)
>+setup_signal_pipe (DBusLoop *loop)
> {
>   DBusError error;
>   DBusWatch *watch;
> 
>   dbus_error_init (&error);
> 
>-  if (!_dbus_full_duplex_pipe (&reload_pipe[0], &reload_pipe[1],
>+  if (!_dbus_full_duplex_pipe (&signal_pipe[0], &signal_pipe[1],
> 			       TRUE, &error))
>     {
>       _dbus_warn ("Unable to create reload pipe: %s\n",
>@@ -211,12 +225,12 @@ setup_reload_pipe (DBusLoop *loop)
>       exit (1);
>     }
> 
>-  _dbus_fd_set_close_on_exec (reload_pipe[0]);
>-  _dbus_fd_set_close_on_exec (reload_pipe[1]);
>+  _dbus_fd_set_close_on_exec (signal_pipe[0]);
>+  _dbus_fd_set_close_on_exec (signal_pipe[1]);
> 
>-  watch = _dbus_watch_new (reload_pipe[RELOAD_READ_END],
>-			   DBUS_WATCH_READABLE, TRUE,
>-			   handle_reload_watch, NULL, NULL);
>+  watch = _dbus_watch_new (signal_pipe[SIGNAL_READ_END],
>+                           DBUS_WATCH_READABLE, TRUE,
>+                           handle_signal_watch, NULL, NULL);
> 
>   if (watch == NULL)
>     {
>@@ -238,13 +252,13 @@ setup_reload_pipe (DBusLoop *loop)
> }
> 
> static void
>-close_reload_pipe (void)
>+close_signal_pipe (void)
> {
>-    _dbus_close_socket (reload_pipe[RELOAD_READ_END], NULL);
>-    reload_pipe[RELOAD_READ_END] = -1;
>+    _dbus_close_socket (signal_pipe[SIGNAL_READ_END], NULL);
>+    signal_pipe[SIGNAL_READ_END] = -1;
> 
>-    _dbus_close_socket (reload_pipe[RELOAD_WRITE_END], NULL);
>-    reload_pipe[RELOAD_WRITE_END] = -1;
>+    _dbus_close_socket (signal_pipe[SIGNAL_WRITE_END], NULL);
>+    signal_pipe[SIGNAL_WRITE_END] = -1;
> }
> 
> int
>@@ -461,8 +475,8 @@ main (int argc, char **argv)
>   /* bus_context_new() closes the print_addr_pipe and
>    * print_pid_pipe
>    */
>-  
>-  setup_reload_pipe (bus_context_get_loop (context));
>+
>+  setup_signal_pipe (bus_context_get_loop (context));
> 
> #ifdef SIGHUP
>   _dbus_set_signal_handler (SIGHUP, signal_handler);
>-- 
>1.6.6.1
>

Actions: View | Splinter Review

Attachments on bug 26303: 34331

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.