Don't drop pending activations when reloading configuration

The reload handling for activation simply dropped all knowledge
of pending activations, which was clearly wrong.  Refactor things
so that reload only reloads directories, server address etc.

Based on a patch originally from Matthias Clasen <mclasen@redhat.com>

diff --git a/bus/activation.c b/bus/activation.c
index 00caac2..0a28df1 100644 (file)
--- a/bus/activation.c
+++ b/bus/activation.c
@@ -735,74 +735,56 @@ out:
   return retval;
 }
 
-BusActivation*
-bus_activation_new (BusContext        *context,
-                    const DBusString  *address,
-                    DBusList         **directories,
-                    DBusError         *error)
+dbus_bool_t
+bus_activation_reload (BusActivation     *activation,
+                       const DBusString  *address,
+                       DBusList         **directories,
+                       DBusError         *error)
 {
-  BusActivation *activation;
   DBusList      *link;
   char          *dir;
-  
-  _DBUS_ASSERT_ERROR_IS_CLEAR (error);
-  
-  activation = dbus_new0 (BusActivation, 1);
-  if (activation == NULL)
-    {
-      BUS_SET_OOM (error);
-      return NULL;
-    }
-  
-  activation->refcount = 1;
-  activation->context = context;
-  activation->n_pending_activations = 0;
-  
+
+  if (activation->server_address != NULL)
+    dbus_free (activation->server_address);
   if (!_dbus_string_copy_data (address, &activation->server_address))
     {
       BUS_SET_OOM (error);
       goto failed;
     }
-  
+
+  if (activation->entries != NULL)
+    _dbus_hash_table_unref (activation->entries);
   activation->entries = _dbus_hash_table_new (DBUS_HASH_STRING, NULL,
                                              (DBusFreeFunction)bus_activation_entry_unref);
   if (activation->entries == NULL)
-    {      
-      BUS_SET_OOM (error);
-      goto failed;
-    }
-
-  activation->pending_activations = _dbus_hash_table_new (DBUS_HASH_STRING, NULL,
-                                                          (DBusFreeFunction)bus_pending_activation_unref);
-
-  if (activation->pending_activations == NULL)
     {
       BUS_SET_OOM (error);
       goto failed;
     }
 
+  if (activation->directories != NULL)
+    _dbus_hash_table_unref (activation->directories);
   activation->directories = _dbus_hash_table_new (DBUS_HASH_STRING, NULL,
                                                   (DBusFreeFunction)bus_service_directory_unref);
-  
-  if (activation->directories == NULL) 
+
+  if (activation->directories == NULL)
     {
       BUS_SET_OOM (error);
       goto failed;
     }
- 
-  /* Load service files */
+
   link = _dbus_list_get_first_link (directories);
   while (link != NULL)
     {
       BusServiceDirectory *s_dir;
-      
+
       dir = _dbus_strdup ((const char *) link->data);
       if (!dir)
         {
           BUS_SET_OOM (error);
           goto failed;
         }
-      
+
       s_dir = dbus_new0 (BusServiceDirectory, 1);
       if (!s_dir)
         {
@@ -813,7 +795,7 @@ bus_activation_new (BusContext        *context,
 
       s_dir->refcount = 1;
       s_dir->dir_c = dir;
-      
+
       s_dir->entries = _dbus_hash_table_new (DBUS_HASH_STRING, NULL,
                                              (DBusFreeFunction)bus_activation_entry_unref);
 
@@ -833,8 +815,8 @@ bus_activation_new (BusContext        *context,
 
       /* only fail on OOM, it is ok if we can't read the directory */
       if (!update_directory (activation, s_dir, error))
-        { 
-          if (dbus_error_has_name (error, DBUS_ERROR_NO_MEMORY)) 
+        {
+          if (dbus_error_has_name (error, DBUS_ERROR_NO_MEMORY))
             goto failed;
           else
             dbus_error_free (error);
@@ -843,10 +825,52 @@ bus_activation_new (BusContext        *context,
       link = _dbus_list_get_next_link (directories, link);
     }
 
+  return TRUE;
+ failed:
+  return FALSE;
+}
+
+BusActivation*
+bus_activation_new (BusContext        *context,
+                    const DBusString  *address,
+                    DBusList         **directories,
+                    DBusError         *error)
+{
+  BusActivation *activation;
+  DBusList      *link;
+  char          *dir;
+
+  _DBUS_ASSERT_ERROR_IS_CLEAR (error);
+
+  activation = dbus_new0 (BusActivation, 1);
+  if (activation == NULL)
+    {
+      BUS_SET_OOM (error);
+      return NULL;
+    }
+
+  activation->refcount = 1;
+  activation->context = context;
+  activation->n_pending_activations = 0;
+
+  if (!bus_activation_reload (activation, address, directories, error))
+    goto failed;
+
+   /* Initialize this hash table once, we don't want to lose pending
+   * activations on reload. */
+  activation->pending_activations = _dbus_hash_table_new (DBUS_HASH_STRING, NULL,
+                                                          (DBusFreeFunction)bus_pending_activation_unref);
+
+  if (activation->pending_activations == NULL)
+    {
+      BUS_SET_OOM (error);
+      goto failed;
+    }
+
   activation->environment = _dbus_hash_table_new (DBUS_HASH_STRING,
                                                   (DBusFreeFunction) dbus_free,
                                                   (DBusFreeFunction) dbus_free);
-  
+
   if (activation->environment == NULL) 
     {
       BUS_SET_OOM (error);

diff --git a/bus/activation.h b/bus/activation.h
index 2dff812..03bfed2 100644 (file)
--- a/bus/activation.h
+++ b/bus/activation.h
@@ -32,6 +32,10 @@ BusActivation* bus_activation_new              (BusContext        *context,
                                                const DBusString  *address,
                                                DBusList         **directories,
                                                DBusError         *error);
+dbus_bool_t bus_activation_reload           (BusActivation     *activation,
+                                               const DBusString  *address,
+                                               DBusList         **directories,
+                                               DBusError         *error);
 BusActivation* bus_activation_ref              (BusActivation     *activation);
 void           bus_activation_unref            (BusActivation     *activation);
 

diff --git a/bus/bus.c b/bus/bus.c
index b370f92..81db7d6 100644 (file)
--- a/bus/bus.c
+++ b/bus/bus.c
@@ -498,21 +498,24 @@ process_config_every_time (BusContext      *context,
       dbus_free(context->servicehelper);
       context->servicehelper = s;
     }
-  
+
   /* Create activation subsystem */
-  new_activation = bus_activation_new (context, &full_address,
-                                       dirs, error);
-  if (new_activation == NULL)
+  if (context->activation)
+    {
+      if (!bus_activation_reload (context->activation, &full_address, dirs, error))
+        goto failed;
+    }
+  else
+    {
+      context->activation = bus_activation_new (context, &full_address, dirs, error);
+    }
+
+  if (context->activation == NULL)
     {
       _DBUS_ASSERT_ERROR_IS_SET (error);
       goto failed;
     }
 
-  if (is_reload)
-    bus_activation_unref (context->activation);
-
-  context->activation = new_activation;
-
   /* Drop existing conf-dir watches (if applicable) */
 
   if (is_reload)