Browse code

- ported stable tm timer race fix (many thanks to Dong Liu for the initial diagnosis and testing of the patch)

Andrei Pelinescu-Onciul authored on 19/09/2003 19:02:08
Showing 3 changed files
... ...
@@ -33,6 +33,7 @@
33 33
  * 2003-03-30  set_kr for requests only (jiri)
34 34
  * 2003-04-04  bug_fix: REQ_IN callback not called for local 
35 35
  *             UAC transactions (jiri)
36
+ * 2003-09-12  timer_link->tg will be set only if EXTRA_DEBUG (andrei)
36 37
  */
37 38
 
38 39
 #include "defs.h"
... ...
@@ -174,8 +175,10 @@ struct cell*  build_cell( struct sip_msg* p_msg )
174 175
 	memset( new_cell, 0, sizeof( struct cell ) );
175 176
 
176 177
 	/* UAS */
178
+#ifdef EXTRA_DEBUG
177 179
 	new_cell->uas.response.retr_timer.tg=TG_RT;
178 180
 	new_cell->uas.response.fr_timer.tg=TG_FR;
181
+#endif
179 182
 	new_cell->uas.response.fr_timer.payload =
180 183
 	new_cell->uas.response.retr_timer.payload = &(new_cell->uas.response);
181 184
 	new_cell->uas.response.my_T=new_cell;
... ...
@@ -205,8 +208,10 @@ struct cell*  build_cell( struct sip_msg* p_msg )
205 208
 		uac=&new_cell->uac[i];
206 209
 		uac->request.my_T = new_cell;
207 210
 		uac->request.branch = i;
211
+#ifdef EXTRA_DEBUG
208 212
 		uac->request.fr_timer.tg = TG_FR;
209 213
 		uac->request.retr_timer.tg = TG_RT;
214
+#endif
210 215
 		uac->request.retr_timer.payload = 
211 216
 		uac->request.fr_timer.payload = &uac->request;
212 217
 		uac->local_cancel=uac->request;
... ...
@@ -226,8 +231,10 @@ struct cell*  build_cell( struct sip_msg* p_msg )
226 231
 	new_cell->dele_tl.payload = new_cell;
227 232
 	new_cell->relaied_reply_branch   = -1;
228 233
 	/* new_cell->T_canceled = T_UNDEFINED; */
234
+#ifdef EXTRA_DEBUG
229 235
 	new_cell->wait_tl.tg=TG_WT;
230 236
 	new_cell->dele_tl.tg=TG_DEL;
237
+#endif
231 238
 
232 239
 	if (!syn_branch) {
233 240
 		if (p_msg) {
... ...
@@ -120,6 +120,12 @@
120 120
 
121 121
 
122 122
 static struct timer_table *timertable=0;
123
+static struct timer detached_timer; /* just to have a value to compare with*/
124
+
125
+#define DETACHED_LIST (&detached_timer)
126
+
127
+#define is_in_timer_list2(_tl) ( (_tl)->timer_list &&  \
128
+									((_tl)->timer_list!=DETACHED_LIST) )
123 129
 
124 130
 int noisy_ctimer=0;
125 131
 
... ...
@@ -299,7 +305,9 @@ inline static void retransmission_handler( void *attr)
299 305
 
300 306
 	id = r_buf->retr_list;
301 307
 	r_buf->retr_list = id < RT_T2 ? id + 1 : RT_T2;
302
-
308
+	
309
+	r_buf->retr_timer.timer_list= NULL; /* set to NULL so that set_timer
310
+										   will work */
303 311
 	set_timer(&(r_buf->retr_timer),id < RT_T2 ? id + 1 : RT_T2 );
304 312
 
305 313
 	DBG("DEBUG: retransmission_handler : done\n");
... ...
@@ -646,7 +654,7 @@ struct timer_link  *check_and_split_time_list( struct timer *timer_list,
646 654
 	end = &timer_list->last_tl;
647 655
 	tl = timer_list->first_tl.next_tl;
648 656
 	while( tl!=end && tl->time_out <= time) {
649
-		tl->timer_list = NULL;
657
+		tl->timer_list = DETACHED_LIST;
650 658
 		tl=tl->next_tl;
651 659
 	}
652 660
 
... ...
@@ -677,7 +685,11 @@ struct timer_link  *check_and_split_time_list( struct timer *timer_list,
677 685
 
678 686
 
679 687
 
680
-/* stop timer */
688
+/* stop timer
689
+ * WARNING: a reset'ed timer will be lost forever
690
+ *  (succesive set_timer won't work unless you're lucky
691
+ *   an catch the race condition, the ideea here is there is no
692
+ *   guarantee you can do anything after a timer_reset)*/
681 693
 void reset_timer( struct timer_link* tl )
682 694
 {
683 695
 	/* disqualify this timer from execution by setting its time_out
... ...
@@ -695,7 +707,14 @@ void reset_timer( struct timer_link* tl )
695 707
 
696 708
 
697 709
 
698
-/* determine timer length and put on a correct timer list */
710
+/* determine timer length and put on a correct timer list
711
+ * WARNING: - don't try to use it to "move" a timer from one list
712
+ *            to another, you'll run into races
713
+ *          - reset_timer; set_timer might not work, a reset'ed timer
714
+ *             has no set_timer guarantee, it might be lost;
715
+ *             same for an expired timer: only it's handler can
716
+ *             set it again, an external set_timer has no guarantee
717
+ */
699 718
 void set_timer( struct timer_link *new_tl, enum lists list_id )
700 719
 {
701 720
 	unsigned int timeout;
... ...
@@ -713,9 +732,19 @@ void set_timer( struct timer_link *new_tl, enum lists list_id )
713 732
 	list= &(timertable->timers[ list_id ]);
714 733
 
715 734
 	lock(list->mutex);
735
+	/* check first if we are on the "detached" timer_routine list,
736
+	 * if so do nothing, the timer is not valid anymore
737
+	 * (sideffect: reset_timer ; set_timer is not safe, a reseted timer
738
+	 *  might be lost, depending on this race condition ) */
739
+	if (new_tl->timer_list==DETACHED_LIST){
740
+		LOG(L_CRIT, "WARNING: set_timer called on a \"detached\" timer"
741
+				" -- ignoring: %p\n", new_tl);
742
+		goto end;
743
+	}
716 744
 	/* make sure I'm not already on a list */
717 745
 	remove_timer_unsafe( new_tl );
718 746
 	add_timer_unsafe( list, new_tl, get_ticks()+timeout);
747
+end:
719 748
 	unlock(list->mutex);
720 749
 }
721 750
 
... ...
@@ -24,6 +24,11 @@
24 24
  * along with this program; if not, write to the Free Software 
25 25
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
26 26
  */
27
+/*
28
+ * History:
29
+ * --------
30
+ *  2003-09-12  timer_link.tg exists only if EXTRA_DEBUG (andrei)
31
+ */
27 32
 
28 33
 
29 34
 #ifndef _TIMER_H
... ...
@@ -39,7 +44,6 @@
39 44
 #define TIMER_DELETED	1
40 45
 
41 46
 
42
-#define is_in_timer_list2(_tl) ( (_tl)->timer_list )
43 47
 
44 48
 /* identifiers of timer lists;*/
45 49
 /* fixed-timer retransmission lists (benefit: fixed timer$
... ...
@@ -63,7 +67,9 @@ typedef struct timer_link
63 67
 	volatile unsigned int       time_out;
64 68
 	void              *payload;
65 69
 	struct timer      *timer_list;
70
+#ifdef EXTRA_DEBUG
66 71
 	enum timer_groups  tg;
72
+#endif
67 73
 }timer_link_type ;
68 74
 
69 75