[patch] pidfile deletion race

[Brian Magnuson]

Hi,
	I just recently got dbus, hal, and hal-device-manager up and running
and I'd first just like to congratulate you all on some great work!

	A question.  Why does the parent write out the pidfile for the child within
_dbus_become_daemon?  If the child detects some problem right away (say, the
messagebus user doesn't exist ;) it tries to delete this pidfile before
exiting, but this races against the creation of this file by the parent.
Leaving a stale pidfile after the parent gets around to writing it and
exits.

	This patch removes any pidfile knowledge from _dbus_become_daemon and does
the pidfile creation after that function is called (or not).

	Does this look sane?  This is the first patch I've sent for anything, ever.
So it might very well be way off.

-Brian

diff -urp dbus/dbus/bus/bus.c dbus-local/dbus/bus/bus.c
--- dbus/dbus/bus/bus.c 2004-03-27 20:17:42.000000000 -0500
+++ dbus-local/dbus/bus/bus.c   2004-03-27 20:39:01.000000000 -0500
@@ -525,26 +525,18 @@ bus_context_new (const DBusString *confi
   /* Now become a daemon if appropriate */
   if (force_fork || bus_config_parser_get_fork (parser))
     {
-      DBusString u;
-
-      if (pidfile)
-        _dbus_string_init_const (&u, pidfile);
-
-      if (!_dbus_become_daemon (pidfile ? &u : NULL, error))
+      if (!_dbus_become_daemon (error))
         goto failed;
     }
-  else
+  if (pidfile != NULL)
     {
-      /* Need to write PID file for ourselves, not for the child process */
-      if (pidfile != NULL)
-        {
-          DBusString u;
+      DBusString u;

-          _dbus_string_init_const (&u, pidfile);
-
-          if (!_dbus_write_pid_file (&u, _dbus_getpid (), error))
-            goto failed;
-        }
+      _dbus_string_init_const (&u, pidfile);
+
+       if (!_dbus_write_pid_file (&u, _dbus_getpid (), error))
+         goto failed;
     }

   /* keep around the pid filename so we can delete it later */
diff -urp dbus/dbus/dbus/dbus-sysdeps.c dbus-local/dbus/dbus/dbus-sysdeps.c
--- dbus/dbus/dbus/dbus-sysdeps.c       2004-03-27 20:17:43.000000000 -0500
+++ dbus-local/dbus/dbus/dbus-sysdeps.c 2004-03-27 20:40:23.000000000 -0500
@@ -3138,8 +3138,7 @@ _dbus_print_backtrace (void)
  * @returns #FALSE on failure
  */
 dbus_bool_t
-_dbus_become_daemon (const DBusString *pidfile,
-                     DBusError        *error)
+_dbus_become_daemon (DBusError        *error)
 {
   const char *s;
   pid_t child_pid;
@@ -3172,7 +3171,6 @@ _dbus_become_daemon (const DBusString *p
        * doesn't have /dev/null we may as well try
        * to continue anyhow
        */
-
       dev_null_fd = open ("/dev/null", O_RDWR);
       if (dev_null_fd >= 0)
         {
@@ -3192,18 +3190,6 @@ _dbus_become_daemon (const DBusString *p
       break;

     default:
-      if (pidfile)
-        {
-          _dbus_verbose ("parent writing pid file\n");
-          if (!_dbus_write_pid_file (pidfile,
-                                     child_pid,
-                                     error))
-            {
-              _dbus_verbose ("pid file write failed, killing child\n");
-              kill (child_pid, SIGTERM);
-              return FALSE;
-            }
-        }
       _dbus_verbose ("parent exiting\n");
       _exit (0);
       break;
diff -urp dbus/dbus/dbus/dbus-sysdeps.h dbus-local/dbus/dbus/dbus-sysdeps.h
--- dbus/dbus/dbus/dbus-sysdeps.h       2004-03-12 09:07:16.000000000 -0500
+++ dbus-local/dbus/dbus/dbus-sysdeps.h 2004-03-27 20:39:54.000000000 -0500
@@ -293,8 +293,7 @@ dbus_bool_t _dbus_close            (int

 void        _dbus_print_backtrace  (void);

-dbus_bool_t _dbus_become_daemon   (const DBusString *pidfile,
-                                   DBusError        *error);
+dbus_bool_t _dbus_become_daemon   (DBusError        *error);
 dbus_bool_t _dbus_write_pid_file  (const DBusString *filename,
                                    unsigned long     pid,
                                    DBusError        *error);