2003-04-10 Havoc Pennington <hp@pobox.com>

	* dbus/dbus-spawn.c (_dbus_spawn_async_with_babysitter): move all
	the possible parent failures before we fork, so that we don't
	fail to create a babysitter after creating the child.

	* bus/activation.c (bus_activation_activate_service): kill child
	if we don't successfully complete the activation.

4 files changed, 75 insertions, 43 deletions

diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index 5a6277de..9595f48a 100644
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -1527,9 +1527,9 @@ dbus_connection_send_with_reply_and_block (DBusConnection     *connection,
   _dbus_get_current_time (&tv_sec, &tv_usec);
   
   if (tv_sec < start_tv_sec)
-    ; /* clock set backward, bail out */
+    _dbus_verbose ("dbus_connection_send_with_reply_and_block(): clock set backward\n");
   else if (connection->disconnect_message_link == NULL)
-    ; /* we're disconnected, bail out */
+    _dbus_verbose ("dbus_connection_send_with_reply_and_block(): disconnected\n");
   else if (tv_sec < end_tv_sec ||
            (tv_sec == end_tv_sec && tv_usec < end_tv_usec))
     {
diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c
index 230c8efe..30f47f00 100644
--- a/dbus/dbus-internals.c
+++ b/dbus/dbus-internals.c
@@ -174,6 +174,8 @@ _dbus_warn (const char *format,
   va_end (args);
 }
 
+static dbus_bool_t verbose_initted = FALSE;
+
 /**
  * Prints a warning message to stderr
  * if the user has enabled verbose mode.
@@ -188,7 +190,6 @@ _dbus_verbose_real (const char *format,
 {
   va_list args;
   static dbus_bool_t verbose = TRUE;
-  static dbus_bool_t initted = FALSE;
   static unsigned long pid;
   
   /* things are written a bit oddly here so that
@@ -198,11 +199,11 @@ _dbus_verbose_real (const char *format,
   if (!verbose)
     return;
   
-  if (!initted)
+  if (!verbose_initted)
     {
       verbose = _dbus_getenv ("DBUS_VERBOSE") != NULL;
       pid = _dbus_getpid ();
-      initted = TRUE;
+      verbose_initted = TRUE;
       if (!verbose)
         return;
     }
@@ -215,6 +216,18 @@ _dbus_verbose_real (const char *format,
 }
 
 /**
+ * Reinitializes the verbose logging code, used
+ * as a hack in dbus-spawn.c so that a child
+ * process re-reads its pid
+ *
+ */
+void
+_dbus_verbose_reset_real (void)
+{
+  verbose_initted = FALSE;
+}
+
+/**
  * Duplicates a string. Result must be freed with
  * dbus_free(). Returns #NULL if memory allocation fails.
  * If the string to be duplicated is #NULL, returns #NULL.
diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h
index b6222fae..b0f41278 100644
--- a/dbus/dbus-internals.h
+++ b/dbus/dbus-internals.h
@@ -53,14 +53,15 @@ DBUS_BEGIN_DECLS;
 #define _DBUS_GNUC_NORETURN
 #endif  /* !__GNUC__ */
 
-void _dbus_warn         (const char *format,
-                         ...) _DBUS_GNUC_PRINTF (1, 2);
-void _dbus_verbose_real (const char *format,
-                         ...) _DBUS_GNUC_PRINTF (1, 2);
-
+void _dbus_warn               (const char *format,
+                               ...) _DBUS_GNUC_PRINTF (1, 2);
+void _dbus_verbose_real       (const char *format,
+                               ...) _DBUS_GNUC_PRINTF (1, 2);
+void _dbus_verbose_reset_real (void);
 
 #ifdef DBUS_ENABLE_VERBOSE_MODE
 #  define _dbus_verbose _dbus_verbose_real
+#  define _dbus_verbose_reset _dbus_verbose_reset_real
 #else
 #  ifdef HAVE_ISO_VARARGS
 #    define _dbus_verbose(...)
@@ -69,6 +70,7 @@ void _dbus_verbose_real (const char *format,
 #  else
 #    error "This compiler does not support varargs macros and thus verbose mode can't be disabled meaningfully"
 #  endif
+#  define _dbus_verbose_reset()
 #endif /* !DBUS_ENABLE_VERBOSE_MODE */
 
 const char* _dbus_strerror (int error_number);
diff --git a/dbus/dbus-spawn.c b/dbus/dbus-spawn.c
index 0e08cd78..c11dba21 100644
--- a/dbus/dbus-spawn.c
+++ b/dbus/dbus-spawn.c
@@ -544,7 +544,6 @@ babysitter_iteration (DBusBabysitter *sitter,
  */
 #define LIVE_CHILDREN(sitter) ((sitter)->socket_to_babysitter >= 0 || (sitter)->error_pipe_from_child >= 0)
 
-
 /**
  * Blocks until the babysitter process gives us the PID of the spawned grandchild,
  * then kills the spawned grandchild.
@@ -559,6 +558,9 @@ _dbus_babysitter_kill_child (DBusBabysitter *sitter)
          sitter->grandchild_pid == -1)
     babysitter_iteration (sitter, TRUE);
 
+  _dbus_verbose ("Got child PID %ld for killing\n",
+                 (long) sitter->grandchild_pid);
+  
   if (sitter->grandchild_pid == -1)
     return; /* child is already dead, or we're so hosed we'll never recover */
 
@@ -826,6 +828,10 @@ do_exec (int                       child_err_report_fd,
   int i, max_open;
 #endif
 
+  _dbus_verbose_reset ();
+  _dbus_verbose ("Child process has PID %lu\n",
+                 _dbus_getpid ());
+  
   if (child_setup)
     (* child_setup) (user_data);
 
@@ -921,6 +927,11 @@ babysit (pid_t grandchild_pid,
 {
   int sigchld_pipe[2];
 
+  /* We don't exec, so we keep parent state, such as the pid that
+   * _dbus_verbose() uses. Reset the pid here.
+   */
+  _dbus_verbose_reset ();
+  
   /* I thought SIGCHLD would just wake up the poll, but
    * that didn't seem to work, so added this pipe.
    * Probably the pipe is more likely to work on busted
@@ -1029,6 +1040,43 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter          **sitter_p,
 
   _dbus_fd_set_close_on_exec (babysitter_pipe[0]);
   _dbus_fd_set_close_on_exec (babysitter_pipe[1]);
+
+  /* Setting up the babysitter is only useful in the parent,
+   * but we don't want to run out of memory and fail
+   * after we've already forked, since then we'd leak
+   * child processes everywhere.
+   */
+  sitter->error_watch = _dbus_watch_new (child_err_report_pipe[READ_END],
+                                         DBUS_WATCH_READABLE,
+                                         TRUE);
+  if (sitter->error_watch == NULL)
+    {
+      dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
+      goto cleanup_and_fail;
+    }
+        
+  if (!_dbus_watch_list_add_watch (sitter->watches,  sitter->error_watch))
+    {
+      dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
+      goto cleanup_and_fail;
+    }
+      
+  sitter->sitter_watch = _dbus_watch_new (babysitter_pipe[0],
+                                          DBUS_WATCH_READABLE,
+                                          TRUE);
+  if (sitter->sitter_watch == NULL)
+    {
+      dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
+      goto cleanup_and_fail;
+    }
+      
+  if (!_dbus_watch_list_add_watch (sitter->watches,  sitter->sitter_watch))
+    {
+      dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
+      goto cleanup_and_fail;
+    }
+
+  _DBUS_ASSERT_ERROR_IS_CLEAR (error);
   
   pid = fork ();
   
@@ -1077,38 +1125,7 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter          **sitter_p,
 	}
     }
   else
-    {
-      /* Parent */
-      sitter->error_watch = _dbus_watch_new (child_err_report_pipe[READ_END],
-                                             DBUS_WATCH_READABLE,
-                                             TRUE);
-      if (sitter->error_watch == NULL)
-        {
-          dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
-          goto cleanup_and_fail;
-        }
-        
-      if (!_dbus_watch_list_add_watch (sitter->watches,  sitter->error_watch))
-        {
-          dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
-          goto cleanup_and_fail;
-        }
-      
-      sitter->sitter_watch = _dbus_watch_new (babysitter_pipe[0],
-                                              DBUS_WATCH_READABLE,
-                                              TRUE);
-      if (sitter->sitter_watch == NULL)
-        {
-          dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
-          goto cleanup_and_fail;
-        }
-      
-      if (!_dbus_watch_list_add_watch (sitter->watches,  sitter->sitter_watch))
-        {
-          dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
-          goto cleanup_and_fail;
-        }
-      
+    {      
       /* Close the uncared-about ends of the pipes */
       close_and_invalidate (&child_err_report_pipe[WRITE_END]);
       close_and_invalidate (&babysitter_pipe[1]);