Browse code

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.)

Miklos Tirpak authored on 02/09/2009 15:03:57
Showing 10 changed files
... ...
@@ -371,12 +371,49 @@ void cfg_destroy(void)
371 371
 	}
372 372
 }
373 373
 
374
-/* per-child process init function */
374
+/* Register num number of child processes that will
375
+ * keep updating their local configuration.
376
+ * This function needs to be called from mod_init
377
+ * before any child process is forked.
378
+ */
379
+void cfg_register_child(int num)
380
+{
381
+	/* Increase the reference counter of the first list item
382
+	 * with the number of child processes.
383
+	 * If the counter was increased after forking then it
384
+	 * could happen that a child process is forked and updates
385
+	 * its local config very fast before the other processes have
386
+	 * a chance to refer to the list item. The result is that the
387
+	 * item is freed by the "fast" child process and the other
388
+	 * processes do not see the beginning of the list and miss
389
+	 * some config changes.
390
+	 */
391
+	atomic_add(&((*cfg_child_cb_first)->refcnt), num);
392
+}
393
+
394
+/* per-child process init function.
395
+ * It needs to be called from the forked process.
396
+ * cfg_register_child() must be called before this function!
397
+ */
375 398
 int cfg_child_init(void)
376 399
 {
377 400
 	/* set the callback list pointer to the beginning of the list */
378 401
 	cfg_child_cb = *cfg_child_cb_first;
379
-	atomic_inc(&cfg_child_cb->refcnt);
402
+
403
+	return 0;
404
+}
405
+
406
+/* Child process init function that can be called
407
+ * without cfg_register_child().
408
+ * Note that the child process may miss some configuration changes.
409
+ */
410
+int cfg_late_child_init(void)
411
+{
412
+	/* set the callback list pointer to the beginning of the list */
413
+	CFG_LOCK();
414
+	atomic_inc(&((*cfg_child_cb_first)->refcnt));
415
+	cfg_child_cb = *cfg_child_cb_first;
416
+	CFG_UNLOCK();
380 417
 
381 418
 	return 0;
382 419
 }
... ...
@@ -124,9 +124,25 @@ int sr_cfg_init(void);
124 124
 /* destroy the memory allocated for the cfg framework */
125 125
 void cfg_destroy(void);
126 126
 
127
-/* per-child process init function */
127
+/* Register num number of child processes that will
128
+ * keep updating their local configuration.
129
+ * This function needs to be called from mod_init
130
+ * before any child process is forked.
131
+ */
132
+void cfg_register_child(int num);
133
+
134
+/* per-child process init function.
135
+ * It needs to be called from the forked process.
136
+ * cfg_register_child() must be called before this function!
137
+ */
128 138
 int cfg_child_init(void);
129 139
 
140
+/* Child process init function that can be called
141
+ * without cfg_register_child().
142
+ * Note that the child process may miss some configuration changes.
143
+ */
144
+int cfg_late_child_init(void);
145
+
130 146
 /* per-child process destroy function
131 147
  * Should be called only when the child process exits,
132 148
  * but SER continues running.
... ...
@@ -334,6 +334,14 @@ update its own local configuration the following way:
334 334
 
335 335
 #include "../../cfg/cfg_struct.h"
336 336
 
337
+int mod_init(void)
338
+{
339
+	/* register the number of children
340
+	 * that will keep updating their local
341
+	 * configuration */
342
+	cfg_register_child(1);
343
+}
344
+
337 345
 void loop_forever(void)
338 346
 {
339 347
 	while(1) {
... ...
@@ -373,8 +381,13 @@ int new_process(void)
373 373
 	if (pid == 0) {
374 374
 		/* This is the child process */
375 375
 
376
-		/* initialize the config framework */
377
-		if (cfg_child_init()) return -1;
376
+		/* initialize the config framework
377
+		 * There is no chance to register the
378
+		 * number of forked processes from mod_init,
379
+		 * hence the late version of ...child_init()
380
+		 * needs to be called.
381
+		 */
382
+		if (cfg_late_child_init()) return -1;
378 383
 
379 384
 		loop_forever(); /* the function may return */
380 385
 
... ...
@@ -1170,6 +1170,16 @@ int main_loop()
1170 1170
 			LOG(L_WARN, "WARNING: using only the first listen address"
1171 1171
 						" (no fork)\n");
1172 1172
 		}
1173
+
1174
+		/* Register the children that will keep updating their
1175
+		 * local configuration */
1176
+		cfg_register_child(
1177
+				1   /* main = udp listener */
1178
+				+ 1 /* timer */
1179
+#ifdef USE_SLOW_TIMER
1180
+				+ 1 /* slow timer */
1181
+#endif
1182
+			);
1173 1183
 		if (do_suid()==-1) goto error; /* try to drop privileges */
1174 1184
 		/* process_no now initialized to zero -- increase from now on
1175 1185
 		   as new processes are forked (while skipping 0 reserved for main
... ...
@@ -1252,6 +1262,16 @@ int main_loop()
1252 1252
 		return udp_rcv_loop();
1253 1253
 	}else{ /* fork: */
1254 1254
 
1255
+		/* Register the children that will keep updating their
1256
+		 * local configuration. (udp/tcp/sctp listeneres
1257
+		 * will be added later.) */
1258
+		cfg_register_child(
1259
+				1   /* timer */
1260
+#ifdef USE_SLOW_TIMER
1261
+				+ 1 /* slow timer */
1262
+#endif
1263
+			);
1264
+
1255 1265
 		for(si=udp_listen;si;si=si->next){
1256 1266
 			/* create the listening socket (for each address)*/
1257 1267
 			/* udp */
... ...
@@ -1265,6 +1285,8 @@ int main_loop()
1265 1265
 					(si->address.af==AF_INET6))
1266 1266
 				sendipv6=si;
1267 1267
 	#endif
1268
+			/* children_no per each socket */
1269
+			cfg_register_child(children_no);
1268 1270
 		}
1269 1271
 #ifdef USE_SCTP
1270 1272
 		if (!sctp_disable){
... ...
@@ -1281,6 +1303,8 @@ int main_loop()
1281 1281
 						(si->address.af==AF_INET6))
1282 1282
 					sendipv6_sctp=si;
1283 1283
 		#endif
1284
+				/* sctp_children_no per each socket */
1285
+				cfg_register_child(sctp_children_no);
1284 1286
 			}
1285 1287
 		}
1286 1288
 #endif /* USE_SCTP */
... ...
@@ -1301,6 +1325,8 @@ int main_loop()
1301 1301
 					sendipv6_tcp=si;
1302 1302
 		#endif
1303 1303
 			}
1304
+			/* the number of sockets does not matter */
1305
+			cfg_register_child(tcp_children_no + 1 /* tcp main */);
1304 1306
 		}
1305 1307
 #ifdef USE_TLS
1306 1308
 		if (!tls_disable && tls_has_init_si()){
... ...
@@ -375,6 +375,10 @@ static int cpl_init(void)
375 375
 		strlower( &cpl_env.realm_prefix );
376 376
 	}
377 377
 
378
+	/* Register a child process that will keep updating
379
+	 * its local configuration */
380
+	cfg_register_child(1);
381
+
378 382
 	return 0;
379 383
 error:
380 384
 	return -1;
... ...
@@ -36,6 +36,7 @@
36 36
 #include "../../ut.h"
37 37
 #include "../../dprint.h"
38 38
 #include "../../pt.h"
39
+#include "../../cfg/cfg_struct.h"
39 40
 #include "ctrl_socks.h"
40 41
 #include "io_listener.h"
41 42
 
... ...
@@ -270,6 +271,8 @@ static int mod_init(void)
270 270
 		/* we will fork */
271 271
 		register_procs(1); /* we will be creating an extra process */
272 272
 		register_fds(fd_no);
273
+		/* The child process will keep updating its local configuration */
274
+		cfg_register_child(1);
273 275
 	}
274 276
 #ifdef USE_FIFO
275 277
 	fifo_rpc_init();
... ...
@@ -33,6 +33,7 @@
33 33
 #include "../../ut.h"
34 34
 #include "../../dprint.h"
35 35
 #include "../../pt.h"
36
+#include "../../cfg/cfg_struct.h"
36 37
 #include "fifo_server.h"
37 38
 #include "fifo.h"
38 39
 
... ...
@@ -82,7 +83,12 @@ static int mod_init(void)
82 82
 	     /* Signal to the core that we will be creating one
83 83
 	      * additional process
84 84
 	      */
85
-	if (fifo) register_procs(1);
85
+	if (fifo) {
86
+		register_procs(1);
87
+		/* The child process will keep updating its
88
+		 * local configuration */
89
+		cfg_register_child(1);
90
+	}
86 91
 	return 0;
87 92
 }
88 93
 
... ...
@@ -280,6 +280,10 @@ static int mod_init(void)
280 280
 		return -1;
281 281
 	}
282 282
 
283
+	/* register nrw + 1 number of children that will keep
284
+	 * updating their local configuration */
285
+	cfg_register_child(nrw + 1);
286
+
283 287
 	DBG("XJAB:mod_init: initialized ...\n");
284 288
 	return 0;
285 289
 }
... ...
@@ -850,8 +854,11 @@ void xjab_check_workers(int mpid)
850 850
 			}
851 851
 
852 852
 
853
-			/* initialize the config framework */
854
-			if (cfg_child_init()) return;
853
+			/* initialize the config framework
854
+			 * The child process was not registered under
855
+			 * the framework during mod_init, therefore the
856
+			 * late version needs to be called. (Miklos) */
857
+			if (cfg_late_child_init()) return;
855 858
 
856 859
 			ctx = db_ctx("jabber");
857 860
 			if (ctx == NULL) goto dberror;
... ...
@@ -109,10 +109,13 @@ natpinger_init(void)
109 109
 		 * Use timer only in single process. For forked SER,
110 110
 		 * use separate process (see natpinger_child_init())
111 111
 		 */
112
-		if (dont_fork)
112
+		if (dont_fork) {
113 113
 			register_timer(natping, NULL, natping_interval);
114
-		else
114
+		} else {
115 115
 			register_procs(1); /* register the separate natpinger process */
116
+			/* The process will keep updating its configuration */
117
+			cfg_register_child(1);
118
+		}
116 119
 
117 120
 		if (natping_method == NULL) {
118 121
 			if (natping_crlf == 0)
... ...
@@ -627,6 +627,10 @@ int global_init()
627 627
 		goto error;
628 628
 	}
629 629
 	*queued_msgs = 0;
630
+	
631
+	/* register nr_of_modems number of child processes that will
632
+	 * update their local configuration */
633
+	cfg_register_child(nr_of_modems);
630 634
 
631 635
 	return 1;
632 636
 error: