[sr-dev] git:master: cfg framework: fix the initialization of child processes

Module: sip-router Branch: master Commit: 5322385b2238c1cff272a42b8fa7e234f36a6f0b URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=5322385… Author: Miklos Tirpak <miklos(a)iptel.org> Committer: Miklos Tirpak <miklos(a)iptel.org> Date: Wed Sep 2 17:03:57 2009 +0200 cfg framework: fix the initialization of child processes The number of child processes that keep updating their local configuration needs to be known before any child process is forked. Before this the child processes increased the reference counter of the callback function list items after forking. If a child process was forked "too fast" then it freed the list before the other processes had a chance to refer to the list item. The result was that some child processes missed the initial configuration changes. (Those changes that had per-child process callback defined.) --- cfg/cfg_struct.c | 41 +++++++++++++++++++++++++++++++++++++++-- cfg/cfg_struct.h | 18 +++++++++++++++++- doc/cfg.txt | 17 +++++++++++++++-- main.c | 26 ++++++++++++++++++++++++++ modules_s/cpl-c/cpl.c | 4 ++++ modules_s/ctl/ctl.c | 3 +++ modules_s/fifo/fifo.c | 8 +++++++- modules_s/jabber/jabber.c | 11 +++++++++-- modules_s/nathelper/natping.c | 7 +++++-- modules_s/sms/sms.c | 4 ++++ 10 files changed, 129 insertions(+), 10 deletions(-) diff --git a/cfg/cfg_struct.c b/cfg/cfg_struct.c index 4f28dfe..c90b12e 100644 --- a/cfg/cfg_struct.c +++ b/cfg/cfg_struct.c @@ -371,12 +371,49 @@ void cfg_destroy(void) } } -/* per-child process init function */ +/* Register num number of child processes that will + * keep updating their local configuration. + * This function needs to be called from mod_init + * before any child process is forked. + */ +void cfg_register_child(int num) +{ + /* Increase the reference counter of the first list item + * with the number of child processes. + * If the counter was increased after forking then it + * could happen that a child process is forked and updates + * its local config very fast before the other processes have + * a chance to refer to the list item. The result is that the + * item is freed by the "fast" child process and the other + * processes do not see the beginning of the list and miss + * some config changes. + */ + atomic_add(&((*cfg_child_cb_first)->refcnt), num); +} + +/* per-child process init function. + * It needs to be called from the forked process. + * cfg_register_child() must be called before this function! + */ int cfg_child_init(void) { /* set the callback list pointer to the beginning of the list */ cfg_child_cb = *cfg_child_cb_first; - atomic_inc(&cfg_child_cb->refcnt); + + return 0; +} + +/* Child process init function that can be called + * without cfg_register_child(). + * Note that the child process may miss some configuration changes. + */ +int cfg_late_child_init(void) +{ + /* set the callback list pointer to the beginning of the list */ + CFG_LOCK(); + atomic_inc(&((*cfg_child_cb_first)->refcnt)); + cfg_child_cb = *cfg_child_cb_first; + CFG_UNLOCK(); return 0; } diff --git a/cfg/cfg_struct.h b/cfg/cfg_struct.h index 975e3aa..f0c2253 100644 --- a/cfg/cfg_struct.h +++ b/cfg/cfg_struct.h @@ -124,9 +124,25 @@ int sr_cfg_init(void); /* destroy the memory allocated for the cfg framework */ void cfg_destroy(void); -/* per-child process init function */ +/* Register num number of child processes that will + * keep updating their local configuration. + * This function needs to be called from mod_init + * before any child process is forked. + */ +void cfg_register_child(int num); + +/* per-child process init function. + * It needs to be called from the forked process. + * cfg_register_child() must be called before this function! + */ int cfg_child_init(void); +/* Child process init function that can be called + * without cfg_register_child(). + * Note that the child process may miss some configuration changes. + */ +int cfg_late_child_init(void); + /* per-child process destroy function * Should be called only when the child process exits, * but SER continues running. diff --git a/doc/cfg.txt b/doc/cfg.txt index 548ef5d..9b306ea 100644 --- a/doc/cfg.txt +++ b/doc/cfg.txt @@ -334,6 +334,14 @@ update its own local configuration the following way: #include "../../cfg/cfg_struct.h" +int mod_init(void) +{ + /* register the number of children + * that will keep updating their local + * configuration */ + cfg_register_child(1); +} + void loop_forever(void) { while(1) { @@ -373,8 +381,13 @@ int new_process(void) if (pid == 0) { /* This is the child process */ - /* initialize the config framework */ - if (cfg_child_init()) return -1; + /* initialize the config framework + * There is no chance to register the + * number of forked processes from mod_init, + * hence the late version of ...child_init() + * needs to be called. + */ + if (cfg_late_child_init()) return -1; loop_forever(); /* the function may return */ diff --git a/main.c b/main.c index c975057..78c8fb5 100644 --- a/main.c +++ b/main.c @@ -1170,6 +1170,16 @@ int main_loop() LOG(L_WARN, "WARNING: using only the first listen address" " (no fork)\n"); } + + /* Register the children that will keep updating their + * local configuration */ + cfg_register_child( + 1 /* main = udp listener */ + + 1 /* timer */ +#ifdef USE_SLOW_TIMER + + 1 /* slow timer */ +#endif + ); if (do_suid()==-1) goto error; /* try to drop privileges */ /* process_no now initialized to zero -- increase from now on as new processes are forked (while skipping 0 reserved for main @@ -1252,6 +1262,16 @@ int main_loop() return udp_rcv_loop(); }else{ /* fork: */ + /* Register the children that will keep updating their + * local configuration. (udp/tcp/sctp listeneres + * will be added later.) */ + cfg_register_child( + 1 /* timer */ +#ifdef USE_SLOW_TIMER + + 1 /* slow timer */ +#endif + ); + for(si=udp_listen;si;si=si->next){ /* create the listening socket (for each address)*/ /* udp */ @@ -1265,6 +1285,8 @@ int main_loop() (si->address.af==AF_INET6)) sendipv6=si; #endif + /* children_no per each socket */ + cfg_register_child(children_no); } #ifdef USE_SCTP if (!sctp_disable){ @@ -1281,6 +1303,8 @@ int main_loop() (si->address.af==AF_INET6)) sendipv6_sctp=si; #endif + /* sctp_children_no per each socket */ + cfg_register_child(sctp_children_no); } } #endif /* USE_SCTP */ @@ -1301,6 +1325,8 @@ int main_loop() sendipv6_tcp=si; #endif } + /* the number of sockets does not matter */ + cfg_register_child(tcp_children_no + 1 /* tcp main */); } #ifdef USE_TLS if (!tls_disable && tls_has_init_si()){ diff --git a/modules_s/cpl-c/cpl.c b/modules_s/cpl-c/cpl.c index 1fca9c1..b521db7 100644 --- a/modules_s/cpl-c/cpl.c +++ b/modules_s/cpl-c/cpl.c @@ -375,6 +375,10 @@ static int cpl_init(void) strlower( &cpl_env.realm_prefix ); } + /* Register a child process that will keep updating + * its local configuration */ + cfg_register_child(1); + return 0; error: return -1; diff --git a/modules_s/ctl/ctl.c b/modules_s/ctl/ctl.c index 730de47..5faec97 100644 --- a/modules_s/ctl/ctl.c +++ b/modules_s/ctl/ctl.c @@ -36,6 +36,7 @@ #include "../../ut.h" #include "../../dprint.h" #include "../../pt.h" +#include "../../cfg/cfg_struct.h" #include "ctrl_socks.h" #include "io_listener.h" @@ -270,6 +271,8 @@ static int mod_init(void) /* we will fork */ register_procs(1); /* we will be creating an extra process */ register_fds(fd_no); + /* The child process will keep updating its local configuration */ + cfg_register_child(1); } #ifdef USE_FIFO fifo_rpc_init(); diff --git a/modules_s/fifo/fifo.c b/modules_s/fifo/fifo.c index 37b12ec..74d43cf 100644 --- a/modules_s/fifo/fifo.c +++ b/modules_s/fifo/fifo.c @@ -33,6 +33,7 @@ #include "../../ut.h" #include "../../dprint.h" #include "../../pt.h" +#include "../../cfg/cfg_struct.h" #include "fifo_server.h" #include "fifo.h" @@ -82,7 +83,12 @@ static int mod_init(void) /* Signal to the core that we will be creating one * additional process */ - if (fifo) register_procs(1); + if (fifo) { + register_procs(1); + /* The child process will keep updating its + * local configuration */ + cfg_register_child(1); + } return 0; } diff --git a/modules_s/jabber/jabber.c b/modules_s/jabber/jabber.c index 10c0ad5..c61bc78 100644 --- a/modules_s/jabber/jabber.c +++ b/modules_s/jabber/jabber.c @@ -280,6 +280,10 @@ static int mod_init(void) return -1; } + /* register nrw + 1 number of children that will keep + * updating their local configuration */ + cfg_register_child(nrw + 1); + DBG("XJAB:mod_init: initialized ...\n"); return 0; } @@ -850,8 +854,11 @@ void xjab_check_workers(int mpid) } - /* initialize the config framework */ - if (cfg_child_init()) return; + /* initialize the config framework + * The child process was not registered under + * the framework during mod_init, therefore the + * late version needs to be called. (Miklos) */ + if (cfg_late_child_init()) return; ctx = db_ctx("jabber"); if (ctx == NULL) goto dberror; diff --git a/modules_s/nathelper/natping.c b/modules_s/nathelper/natping.c index d4735af..4b08b3d 100644 --- a/modules_s/nathelper/natping.c +++ b/modules_s/nathelper/natping.c @@ -109,10 +109,13 @@ natpinger_init(void) * Use timer only in single process. For forked SER, * use separate process (see natpinger_child_init()) */ - if (dont_fork) + if (dont_fork) { register_timer(natping, NULL, natping_interval); - else + } else { register_procs(1); /* register the separate natpinger process */ + /* The process will keep updating its configuration */ + cfg_register_child(1); + } if (natping_method == NULL) { if (natping_crlf == 0) diff --git a/modules_s/sms/sms.c b/modules_s/sms/sms.c index db4ea66..5b57372 100644 --- a/modules_s/sms/sms.c +++ b/modules_s/sms/sms.c @@ -627,6 +627,10 @@ int global_init() goto error; } *queued_msgs = 0; + + /* register nr_of_modems number of child processes that will + * update their local configuration */ + cfg_register_child(nr_of_modems); return 1; error: