- variable timer fix: variable timers (avps) won't be exteneded anymore in

 some special situations. Tested with random generated timer values; minimal
 perfromance impact. Performance maniacs can turn the new code off
  modparam("tm", "var_timers", 0).
 Thanks to Bogdan Iancu  <[email protected]> for the excelent bug report.

Makefile.defs

@@ -59,7 +59,7 @@ MAIN_NAME=ser
 VERSION = 0
 PATCHLEVEL = 9
 SUBLEVEL = 7
-EXTRAVERSION = -pre7
+EXTRAVERSION = -pre8
 
 RELEASE=$(VERSION).$(PATCHLEVEL).$(SUBLEVEL)$(EXTRAVERSION)
 OS = $(shell uname -s | sed -e s/SunOS/solaris/ | tr "[A-Z]" "[a-z]")

modules/tm/t_cancel.c

@@ -138,7 +138,9 @@ static void cancel_all_branches(struct cell *t)
 {
     int i;
     int reply_recved=0;
-    
+    int fr_locked;
+   
+    fr_locked=0;
     /* cancel pending client transactions, if any */
     for( i=0 ; i<t->nr_of_outgoings ; i++ ){
 
@@ -147,13 +149,23 @@ static void cancel_all_branches(struct cell *t)
 	    
 	    /* stop_rb_timers(&t->uac[i].request); */
 	    reset_timer( &t->uac[i].request.retr_timer );
-	    reset_timer( &t->uac[i].request.fr_timer );
+	    if (var_timers){
+	    	if (fr_locked==0){
+			lock_fr_timers();
+			fr_locked=1;
+	    	}
+	    	del_fr_timer_unsafe( &t->uac[i].request.fr_timer );
+	    }else{
+	    	del_fr_timer( &t->uac[i].request.fr_timer );
+	    }
 	}
 	else {
 
 	    reply_recved++;
 	}
     }
+    if (fr_locked)
+	unlock_fr_timers();
 
     if(!reply_recved){
 	

modules/tm/t_funcs.c

@@ -133,10 +133,16 @@ int t_release_transaction( struct cell *trans )
 {
 	set_kr(REQ_RLSD);
 
-	reset_timer( & trans->uas.response.fr_timer );
 	reset_timer( & trans->uas.response.retr_timer );
-
-	cleanup_uac_timers( trans );
+	if (var_timers){
+		lock_fr_timers();
+			del_fr_timer_unsafe( & trans->uas.response.fr_timer );
+			cleanup_uac_timers_unsafe( trans );
+		unlock_fr_timers();
+	}else{
+		del_fr_timer( & trans->uas.response.fr_timer );
+		cleanup_uac_timers( trans );
+	}
 	
 	put_on_wait( trans );
 	return 1;
@@ -395,7 +401,7 @@ static inline int avp2timer(unsigned int* timer, int type, int_str name)
 
 int fr_avp2timer(unsigned int* timer)
 {
-	if (fr_timer_avp.n!=0)
+	if (var_timers && fr_timer_avp.n!=0)
 		return avp2timer( timer, fr_timer_avp_type, fr_timer_avp);
 	else
 		return 1;
@@ -404,7 +410,7 @@ int fr_avp2timer(unsigned int* timer)
 
 int fr_inv_avp2timer(unsigned int* timer)
 {
-	if (fr_inv_timer_avp.n!=0)
+	if (var_timers && fr_inv_timer_avp.n!=0)
 		return avp2timer( timer, fr_inv_timer_avp_type, fr_inv_timer_avp);
 	else
 		return 1;

modules/tm/t_fwd.c

@@ -349,7 +349,7 @@ void e2e_cancel( struct sip_msg *cancel_msg,
 				      * retransmission timers
 				      */
 				reset_timer(&t_invite->uac[i].request.retr_timer);
-				reset_timer(&t_invite->uac[i].request.fr_timer);
+				del_fr_timer(&t_invite->uac[i].request.fr_timer);
 
 				/* Generate faked reply */
 				LOCK_REPLIES(t_invite);

modules/tm/t_reply.c

@@ -66,6 +66,7 @@
  *              the request (bogdan)
  *  2005-09-01  reverted to the old way of checking response.dst.send_sock
  *               in t_retransmit_reply & reply_light (andrei)
+ *  2006-12-27  replaced reset(fr_timer) with del_fr_timer(...)  (andrei)
  */
 
 
@@ -947,18 +948,30 @@ void set_final_timer( /* struct s_table *h_table, */ struct cell *t )
 	put_on_wait(t);
 }
 
-void cleanup_uac_timers( struct cell *t )
+
+/* must be called with the FR TIMER list lock held, if
+ * var timers are used */
+void cleanup_uac_timers_unsafe( struct cell *t )
 {
 	int i;
 
 	/* reset FR/retransmission timers */
 	for (i=0; i<t->nr_of_outgoings; i++ )  {
 		reset_timer( &t->uac[i].request.retr_timer );
-		reset_timer( &t->uac[i].request.fr_timer );
+		del_fr_timer_unsafe( &t->uac[i].request.fr_timer );
 	}
-	DBG("DEBUG: cleanup_uac_timers: RETR/FR timers reset\n");
 }
 
+void cleanup_uac_timers( struct cell *t )
+{
+
+	lock_fr_timers();
+		cleanup_uac_timers_unsafe(t);
+	unlock_fr_timers();
+}
+
+
+
 static int store_reply( struct cell *trans, int branch, struct sip_msg *rpl)
 {
 #		ifdef EXTRA_DEBUG
@@ -1282,7 +1295,7 @@ int reply_received( struct sip_msg  *p_msg )
 		     /* ... then just stop timers */
 		reset_timer( &uac->local_cancel.retr_timer);
 		if ( msg_status >= 200 ) {
-				reset_timer( &uac->local_cancel.fr_timer);
+				del_fr_timer( &uac->local_cancel.fr_timer);
 		}
 		DBG("DEBUG: reply to local CANCEL processed\n");
 		goto done;
@@ -1294,7 +1307,7 @@ int reply_received( struct sip_msg  *p_msg )
 	
 	     /* stop final response timer only if I got a final response */
 	if ( msg_status >= 200 ) {
-		reset_timer( &uac->request.fr_timer);
+		del_fr_timer(&uac->request.fr_timer);
 	}
 
 	     /* acknowledge negative INVITE replies (do it before detailed

modules/tm/t_reply.h

@@ -125,6 +125,7 @@ enum rps local_reply( struct cell *t, struct sip_msg *p_msg, int branch,
 void set_final_timer( /* struct s_table *h_table,*/ struct cell *t );
 
 void cleanup_uac_timers( struct cell *t );
+void cleanup_uac_timers_unsafe( struct cell *t );
 
 void on_negative_reply( struct cell* t, struct sip_msg* msg,
 	int code, void *param  );

modules/tm/timer.c

@@ -98,6 +98,8 @@
  *  2003-06-27  timers are not unlinked if timerlist is 0 (andrei)
  *  2004-02-13  t->is_invite, t->local, t->noisy_ctimer replaced;
  *              timer_link.payload removed (bogdan)
+ *  2006-11-27  added del_fr_timer(): fr timers are immediately removed
+ *              from the FR* lists (andrei)
  */
 
 #include "defs.h"
@@ -130,7 +132,8 @@ static struct timer detached_timer; /* just to have a value to compare with*/
 									((_tl)->timer_list!=DETACHED_LIST) )
 
 int noisy_ctimer=0;
-
+int var_timers=1; /* set to 0 to disable variable timers &&
+					timer loading from avps */
 
 int timer_group[NR_OF_TIMER_LISTS] = 
 {
@@ -322,7 +325,7 @@ inline static void retransmission_handler( struct timer_link *retr_tl )
 				"request resending (t=%p, %.9s ... )\n", 
 				r_buf->my_T, r_buf->buffer);
 			if (SEND_BUFFER( r_buf )==-1) {
-				reset_timer( &r_buf->fr_timer );
+				del_fr_timer( &r_buf->fr_timer );
 				fake_reply(r_buf->my_T, r_buf->branch, 503 );
 				return;
 			}
@@ -436,7 +439,17 @@ void cleanup_localcancel_timers( struct cell *t )
 	int i;
 	for (i=0; i<t->nr_of_outgoings; i++ )  {
 		reset_timer(  &t->uac[i].local_cancel.retr_timer );
-		reset_timer(  &t->uac[i].local_cancel.fr_timer );
+	}
+	if (var_timers){
+		lock(timertable->timers[FR_TIMER_LIST].mutex);
+			for (i=0; i<t->nr_of_outgoings; i++ )  {
+				del_fr_timer_unsafe(&t->uac[i].local_cancel.fr_timer);
+			}
+		unlock(timertable->timers[FR_TIMER_LIST].mutex);
+	}else{
+		for (i=0; i<t->nr_of_outgoings; i++ )  {
+			del_fr_timer(&t->uac[i].local_cancel.fr_timer);
+		}
 	}
 }
 
@@ -510,6 +523,21 @@ struct timer_table *get_timertable()
 }
 
 
+
+void lock_fr_timers()
+{
+	lock(timertable->timers[FR_TIMER_LIST].mutex);
+}
+
+
+
+void unlock_fr_timers()
+{
+	unlock(timertable->timers[FR_TIMER_LIST].mutex);
+}
+
+
+
 void unlink_timer_lists()
 {
 	struct timer_link  *tl, *end, *tmp;
@@ -778,6 +806,54 @@ void reset_timer( struct timer_link* tl )
 
 
 
+/* removes a timer from the FR_TIMER_LIST or FR_INV_TIMER_LIST, unsafe version,
+ *  (it allows immediate delete of a fr timer => solves the race with
+ *   variables timers inserted after longer deleted timers)
+ * WARNING: - don't try to use it to "move" a timer from one list
+ *            to another, you'll run into races
+ *          - must be calles with FR_TIMER lock held!
+ * see del_fr_timer()
+ */
+void del_fr_timer_unsafe( struct timer_link *tl)
+{
+	/* check first if var. timers are really used and we  are on  the
+	 * "detached" timer_routine list (the fr
+	 * handle is executing or  timer_routine prepares to execute it).
+	 * if so do nothing, except reseting the timer to TIMER_DELETED
+	 *  (just to give us a change at racing with timer_routine, if 
+	 *  TIMER_DELETED is set and the fr handle is not already executing =>
+	 *   it will not be called anymore)
+	 */
+	if (var_timers && tl->timer_list!=DETACHED_LIST){
+		remove_timer_unsafe(tl); /* safe to call for null list */
+	}else{
+		reset_timer(tl);
+	}
+}
+
+
+
+/* removes a timer from the FR_TIMER_LIST or FR_INV_TIMER_LIST
+ *  (it allows immediate delete of a fr timer => solves the race with
+ *   variables timers inserted after longer deleted timers)
+ * WARNING: - don't try to use it to "move" a timer from one list
+ *            to another, you'll run into races
+ */
+void del_fr_timer( struct timer_link *tl)
+{
+	if (var_timers){
+		/* the FR lock is common/shared by both FR_INV_TIMER_LIST 
+		 * and FR_TIMER_LIST, so we must lock only one of them */
+		lock(timertable->timers[FR_TIMER_LIST].mutex);
+			del_fr_timer_unsafe(tl);
+		unlock(timertable->timers[FR_TIMER_LIST].mutex);
+	}else{
+		/* use the older faster method */
+		reset_timer(tl);
+	}
+}
+
+
 
 /* determine timer length and put on a correct timer list
  * WARNING: - don't try to use it to "move" a timer from one list

modules/tm/timer.h

@@ -95,7 +95,7 @@ struct timer_table
 
 extern int timer_group[NR_OF_TIMER_LISTS];
 extern unsigned int timer_id2timeout[NR_OF_TIMER_LISTS];
-
+extern int var_timers;
 
 
 struct timer_table * tm_init_timers();
@@ -109,6 +109,9 @@ struct timer_link  *check_and_split_time_list( struct timer*, int);
 */
 
 void reset_timer( struct timer_link* tl );
+/* remove a timer from FR_TIMER_LIST or FR_INV_TIMER_LIST */
+void del_fr_timer( struct timer_link *tl);
+void del_fr_timer_unsafe( struct timer_link *tl);
 /* determine timer length and put on a correct timer list */
 void set_timer( struct timer_link *new_tl, enum lists list_id, unsigned int* ext_timeout );
 /* similar to set_timer, except it allows only one-time
@@ -117,6 +120,8 @@ void set_1timer( struct timer_link *new_tl, enum lists list_id, unsigned int* ex
 /*void unlink_timers( struct cell *t );*/
 void timer_routine(unsigned int, void*);
 
+void lock_fr_timers();
+void unlock_fr_timers();
 
 struct timer_table *get_timertable();
 

modules/tm/tm.c

@@ -274,7 +274,8 @@ static param_export_t params[]={
 	{"fr_timer_avp",        STR_PARAM, &fr_timer_param                       },
 	{"fr_inv_timer_avp",    STR_PARAM, &fr_inv_timer_param                   },
 	{"tw_append",           STR_PARAM|USE_FUNC_PARAM, (void*)parse_tw_append },
-        {"pass_provisional_replies", INT_PARAM, &pass_provisional_replies        },
+    {"pass_provisional_replies", INT_PARAM, &pass_provisional_replies        },
+    {"var_timers",          INT_PARAM, &var_timers                           },
 	{0,0,0}
 };