- Call the per-child process callback functions even if the

config variables are changed before forking, so the modules/core
will not miss the change.

- fixing a very unlikely bug: when a module kept forking and destroying
new processes runtime (only jabber module does so), and two variables
were changed after the forked process called called cfg_update()
and before exited, and both variables had per-child process callback,
and all the other child processes updated their own local config faster
then this one, the list of the callbacks was not released, thus the
memory was not freed.

diff --git a/cfg/cfg_ctx.c b/cfg/cfg_ctx.c
index eac562a..df7458d 100644 (file)
--- a/cfg/cfg_ctx.c
+++ b/cfg/cfg_ctx.c
@@ -303,16 +303,16 @@ int cfg_set_now(cfg_ctx_t *ctx, str *group_name, str *var_name,
 
        }
 
 
        }
 

-       if (cfg_shmized) {
-               if (var->def->on_set_child_cb) {
-                       child_cb = cfg_child_cb_new(var_name,
-                                               var->def->on_set_child_cb);
-                       if (!child_cb) {
-                               LOG(L_ERR, "ERROR: cfg_set_now(): not enough shm memory\n");
-                               goto error0;
-                       }
+       if (var->def->on_set_child_cb) {
+               child_cb = cfg_child_cb_new(var_name,
+                                       var->def->on_set_child_cb);
+               if (!child_cb) {
+                       LOG(L_ERR, "ERROR: cfg_set_now(): not enough shm memory\n");
+                       goto error0;

                }
                }

+       }

 
 

+       if (cfg_shmized) {

                /* make sure that nobody else replaces the global config
                while the new one is prepared */
                CFG_WRITER_LOCK();
                /* make sure that nobody else replaces the global config
                while the new one is prepared */
                CFG_WRITER_LOCK();


@@ -381,6 +381,11 @@ int cfg_set_now(cfg_ctx_t *ctx, str *group_name, str *var_name,
                /* flag the variable because there is no need
                to shmize it again */
                var->flag |= cfg_var_shmized;
                /* flag the variable because there is no need
                to shmize it again */
                var->flag |= cfg_var_shmized;

+
+               /* the global config does not have to be replaced,
+               but the child callback has to be installed, otherwise the
+               child processes will miss the change */
+               cfg_install_child_cb(child_cb, child_cb);

        }
 
        if (val_type == CFG_VAR_INT)
        }
 
        if (val_type == CFG_VAR_INT)

diff --git a/cfg/cfg_struct.c b/cfg/cfg_struct.c
index 25e9c54..1eca977 100644 (file)
--- a/cfg/cfg_struct.c
+++ b/cfg/cfg_struct.c
@@ -385,28 +385,50 @@ int cfg_child_init(void)
  */
 void cfg_child_destroy(void)
 {
  */
 void cfg_child_destroy(void)
 {

+       cfg_child_cb_t  *prev_cb;
+

        /* unref the local config */
        if (cfg_local) {
                CFG_UNREF(cfg_local);
                cfg_local = NULL;
        }
 
        /* unref the local config */
        if (cfg_local) {
                CFG_UNREF(cfg_local);
                cfg_local = NULL;
        }
 

-       /* unref the per-process callback list */
-       if (atomic_dec_and_test(&cfg_child_cb->refcnt)) {
-               /* No more pocess refers to this callback.
-               Did this process block the deletion,
-               or is there any other process that has not
-               reached prev_cb yet? */
-               CFG_LOCK();
-               if (*cfg_child_cb_first == cfg_child_cb) {
-                       /* yes, this process was blocking the deletion */
-                       *cfg_child_cb_first = cfg_child_cb->next;
-                       CFG_UNLOCK();
-                       shm_free(cfg_child_cb);
+       if (!cfg_child_cb) return;
+
+       /* The lock must be held to make sure that the global config
+       is not replaced meantime, and the other child processes do not
+       leave the old value of *cfg_child_cb_last. Otherwise it could happen,
+       that all the other processes move their own cfg_child_cb pointer before
+       this process reaches *cfg_child_cb_last, though, it is very unlikely. */
+       CFG_LOCK();
+
+       /* go through the list and check whether there is any item that
+       has to be freed (similar to cfg_update_local(), but without executing
+       the callback functions) */
+       while (cfg_child_cb != *cfg_child_cb_last) {
+               prev_cb = cfg_child_cb;
+               cfg_child_cb = cfg_child_cb->next;
+               atomic_inc(&cfg_child_cb->refcnt);
+               if (atomic_dec_and_test(&prev_cb->refcnt)) {
+                       /* No more pocess refers to this callback.
+                       Did this process block the deletion,
+                       or is there any other process that has not
+                       reached prev_cb yet? */
+                       if (*cfg_child_cb_first == prev_cb) {
+                               /* yes, this process was blocking the deletion */
+                               *cfg_child_cb_first = cfg_child_cb;
+                               shm_free(prev_cb);
+                       }

                } else {
                } else {

-                       CFG_UNLOCK();
+                       /* no need to continue, because there is at least
+                       one process that stays exactly at the same point
+                       in the list, so it will free the items later */
+                       break;

                }
        }
                }
        }

+       atomic_dec(&cfg_child_cb->refcnt);
+
+       CFG_UNLOCK();

        cfg_child_cb = NULL;
 }
 
        cfg_child_cb = NULL;
 }
 


@@ -480,6 +502,18 @@ cfg_block_t *cfg_clone_global(void)
        return block;
 }
 
        return block;
 }
 

+/* append new callbacks to the end of the child callback list
+ *
+ * WARNING: the function is unsafe, either hold CFG_LOCK(),
+ * or call the function before forking
+ */
+void cfg_install_child_cb(cfg_child_cb_t *cb_first, cfg_child_cb_t *cb_last)
+{
+       /* add the new callbacks to the end of the linked-list */
+       (*cfg_child_cb_last)->next = cb_first;
+       *cfg_child_cb_last = cb_last;
+}
+

 /* installs a new global config
  *
  * replaced is an array of strings that must be freed together
 /* installs a new global config
  *
  * replaced is an array of strings that must be freed together


@@ -499,11 +533,8 @@ void cfg_install_global(cfg_block_t *block, char **replaced,
        CFG_REF(block);
        *cfg_global = block;
 
        CFG_REF(block);
        *cfg_global = block;
 

-       if (cb_first) {
-               /* add the new callbacks to the end of the linked-list */
-               (*cfg_child_cb_last)->next = cb_first;
-               *cfg_child_cb_last = cb_last;
-       }
+       if (cb_first)
+               cfg_install_child_cb(cb_first, cb_last);

 
        CFG_UNLOCK();
 
 
        CFG_UNLOCK();
 

diff --git a/cfg/cfg_struct.h b/cfg/cfg_struct.h
index 3afca59..95af1b7 100644 (file)
--- a/cfg/cfg_struct.h
+++ b/cfg/cfg_struct.h
@@ -262,6 +262,13 @@ cfg_block_t *cfg_clone_global(void);
 /* clones a string to shared memory */
 int cfg_clone_str(str *src, str *dst);
 
 /* clones a string to shared memory */
 int cfg_clone_str(str *src, str *dst);
 

+/* append new callbacks to the end of the child callback list
+ *
+ * WARNING: the function is unsafe, either hold CFG_LOCK(),
+ * or call the function before forking
+ */
+void cfg_install_child_cb(cfg_child_cb_t *cb_first, cfg_child_cb_t *cb_last);
+

 /* installs a new global config
  *
  * replaced is an array of strings that must be freed together
 /* installs a new global config
  *
  * replaced is an array of strings that must be freed together