Fix reload of timer routes

Use a separate, dynamic list of timer tasks that can be update by other proc than the timer itself (which has no info on the script).
The proc doing the reload, upon success, re-generates the list of timer tasks corresponding to the new set of timer routes.
NOTE: a more complex logic is needed when purging the old set of timer tasks, as they may still be under execution - we use here a pending list where we wait their completion.

(cherry picked from commit fce0eae)
(cherry picked from commit 0950670)

cfg_reload.c

@@ -580,6 +580,8 @@ int reload_routing_script(void)
 
 	send_cmd_to_all_procs( routes_switch_per_proc );
 
+	register_route_timers();
+
 	/* ready for a new reload :) */
 	reset_script_reload_ctx();
 

route.c

@@ -89,6 +89,7 @@ extern int return_code;
  */
 struct os_script_routes* new_sroutes_holder(void)
 {
+	static unsigned int sr_version = 0;
 	struct os_script_routes *sr;
 
 	sr = (struct os_script_routes *) pkg_malloc
@@ -102,6 +103,8 @@ struct os_script_routes* new_sroutes_holder(void)
 	sr->request[DEFAULT_RT].name = "0";
 	sr->onreply[DEFAULT_RT].name = "0";
 
+	sr->version = ++sr_version;
+
 	return sr;
 }
 

route.h

@@ -69,6 +69,8 @@ struct os_script_routes {
 	struct script_timer_route timer[TIMER_RT_NO];
 	/* event route */
 	struct script_route event[EVENT_RT_NO];
+	/* script version (due to reload) */
+	unsigned int version;
 };
 
 

timer.c

@@ -37,6 +37,7 @@
  * special defines enabled (mainly sys/types.h) */
 #include "reactor.h"
 #include "pt_load.h"
+#include "locking.h"
 
 #include <unistd.h>
 #include <fcntl.h>
@@ -72,6 +73,10 @@ static unsigned short timer_id=0;
 static int            timer_pipe[2];
 static struct scaling_profile *s_profile=NULL;
 
+static gen_lock_t      *tr_list_lock = NULL;
+static struct os_timer **tr_timer_list = NULL;
+static struct os_timer **tr_timer_pending = NULL;
+
 int timer_fd_out = -1 ;
 char *timer_auto_scaling_profile = NULL;
 int timer_workers_no = 1;
@@ -151,6 +156,33 @@ int init_timer(void)
 		auto_scaling_enabled = 1;
 	}
 
+	/* lock to protect the list of timer task for timer routes */
+	tr_list_lock = lock_alloc();
+	if (tr_list_lock==0) {
+		LM_ERR("failed to alloc lock\n");
+		return E_UNSPEC;
+	}
+
+	if (lock_init(tr_list_lock)==0) {
+		LM_ERR("failed to init lock\n");
+		return E_UNSPEC;
+	}
+
+	tr_timer_list = (struct os_timer**)shm_malloc(sizeof(struct os_timer*));
+	if (tr_timer_list==NULL) {
+		LM_ERR("failed to alloc timer holder\n");
+		return E_UNSPEC;
+	}
+	*tr_timer_list = NULL;
+
+	tr_timer_pending = (struct os_timer**)shm_malloc(sizeof(struct os_timer*));
+	if (tr_timer_pending==NULL) {
+		LM_ERR("failed to alloc timer pending holder\n");
+		return E_UNSPEC;
+	}
+	*tr_timer_pending = NULL;
+
+
 	return 0;
 }
 
@@ -229,14 +261,25 @@ int register_utimer(char *label, utimer_function f, void* param,
 }
 
 
+struct timer_route_param {
+	unsigned int idx;
+	unsigned int version;
+};
+
 void route_timer_f(unsigned int ticks, void* param)
 {
-	struct action* a = (struct action*)param;
-	struct sip_msg* req= NULL;
+	struct timer_route_param *tr=(struct timer_route_param *)param;
+	struct sip_msg *req;
 	int old_route_type;
 
-	if(a == NULL) {
-		LM_ERR("NULL action\n");
+	if (tr->version!=sroutes->version) {
+		LM_WARN("timer route triggering received for an old cfg version "
+			"%d<>%d\n",tr->version, sroutes->version);
+		return;
+	}
+
+	if(sroutes->timer[tr->idx].a == NULL) {
+		LM_ERR("NULL actions for timer_route %d\n", tr->idx);
 		return;
 	}
 
@@ -247,7 +290,7 @@ void route_timer_f(unsigned int ticks, void* param)
 	}
 
 	swap_route_type(old_route_type, TIMER_ROUTE);
-	run_top_route(a, req);
+	run_top_route( sroutes->timer[tr->idx].a, req);
 	set_route_type(old_route_type);
 
 	/* clean whatever extra structures were added by script functions */
@@ -258,29 +301,69 @@ void route_timer_f(unsigned int ticks, void* param)
 }
 
 
+/* the function will check the timer routes from the current process,
+ * so be carefull where you are running it from */
 int register_route_timers(void)
 {
-	struct os_timer* t;
+	struct timer_route_param *tr_param;
+	struct os_timer *t, *p;
 	int i;
 
-	if(sroutes->timer[0].a == NULL)
-		return 0;
+#define move_to_pending( _t) \
+	while(_t) { \
+		p = (_t)->next; \
+		if ((_t)->trigger_time) { \
+			(_t)->next = *tr_timer_pending; \
+			*tr_timer_pending = (_t); \
+		} else { \
+			shm_free( (_t)->t_param ); \
+			shm_free( (_t) ); \
+		} \
+		(_t) = p; \
+	}
+
+	lock_get(tr_list_lock);
 
-	/* register the routes */
-	for(i = 0; i< TIMER_RT_NO; i++)
+	/* handle the pending list, remove whatever already finished,
+	 * otherwise put back into pending */
+	t = *tr_timer_pending;
+	*tr_timer_pending = NULL;
+	move_to_pending( t);
+
+	/* handle the existing list -> free if done or move to pending if
+	 * the job is still under execution (for sure triggering cannot be
+	 * done anymore as the have the lock here) */
+	t = *tr_timer_list;
+	move_to_pending( t);
+	*tr_timer_list = NULL;
+
+	/* convert timer routes to jobs */
+	for(i = 0; i<TIMER_RT_NO && sroutes->timer[i].a ; i++)
 	{
-		if(sroutes->timer[i].a == NULL)
-			return 0;
-		t = new_os_timer( "timer_route", 0, route_timer_f, sroutes->timer[i].a,
+		LM_DBG("registering timer route at %d secs\n",
+			sroutes->timer[i].interval);
+
+		tr_param = (struct timer_route_param*)
+			shm_malloc( sizeof(struct timer_route_param) );
+		if (tr_param==NULL) {
+			LM_ERR("no more mem, skipping route timer\n");
+		} else {
+			tr_param->idx = i;
+			tr_param->version = sroutes->version;
+			t = new_os_timer( "timer_route", 0, route_timer_f, (void*)tr_param,
 				sroutes->timer[i].interval);
-		if (t==NULL)
-			return E_OUT_OF_MEM;
-
-		/* insert it into the list*/
-		t->next = timer_list;
-		timer_list = t;
+			if (t==NULL) {
+				LM_ERR("no more mem, skipping route timer\n");
+			} else {
+				/* insert it into the list*/
+				t->next = *tr_timer_list;
+				*tr_timer_list = t;
+			}
+		}
 	}
 
+	lock_release(tr_list_lock);
+
 	return 1;
 }
 
@@ -458,21 +541,11 @@ static void run_timer_process( void )
 			compute_wait_with_drift(comp_tv);
 			tv = comp_tv;
 			select( 0, 0, 0, 0, &tv);
-			timer_ticker( timer_list);
-
-			drift += ((utime_t)comp_tv.tv_sec*1000000+comp_tv.tv_usec > (*ijiffies-ij)) ?
-					0 : *ijiffies-ij - ((utime_t)comp_tv.tv_sec*1000000+comp_tv.tv_usec);
-		}
 
-	} else
-	if (timer_list==NULL) {
-		/* only UTIMERs, ticking at UTIMER_TICK */
-		for( ; ; ) {
-			ij = *ijiffies;
-			compute_wait_with_drift(comp_tv);
-			tv = comp_tv;
-			select( 0, 0, 0, 0, &tv);
-			utimer_ticker( utimer_list);
+			timer_ticker( timer_list);
+			lock_get(tr_list_lock);
+			timer_ticker( *tr_timer_list);
+			lock_release(tr_list_lock);
 
 			drift += ((utime_t)comp_tv.tv_sec*1000000+comp_tv.tv_usec > (*ijiffies-ij)) ?
 					0 : *ijiffies-ij - ((utime_t)comp_tv.tv_sec*1000000+comp_tv.tv_usec);
@@ -487,6 +560,9 @@ static void run_timer_process( void )
 			tv = comp_tv;
 			select( 0, 0, 0, 0, &tv);
 			timer_ticker( timer_list);
+			lock_get(tr_list_lock);
+			timer_ticker( *tr_timer_list);
+			lock_release(tr_list_lock);
 			utimer_ticker( utimer_list);
 
 			drift += ((utime_t)comp_tv.tv_sec*1000000+comp_tv.tv_usec > (*ijiffies-ij)) ?
@@ -503,6 +579,9 @@ static void run_timer_process( void )
 			utimer_ticker(utimer_list);
 			if (cnt==multiple) {
 				timer_ticker(timer_list);
+				lock_get(tr_list_lock);
+				timer_ticker( *tr_timer_list);
+				lock_release(tr_list_lock);
 				cnt = 0;
 			}