Browse code

- fix for the race condition reported by Dong Liu (many thanks to Dong Liu for the intitial diagnosis and patch testing) - tm cleanups: timer_link.tg exists/is set only if EXTRA_DEBUG

Andrei Pelinescu-Onciul authored on 19/09/2003 18:59:37
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");
... ...
@@ -651,7 +659,7 @@ struct timer_link  *check_and_split_time_list( struct timer *timer_list,
651 659
 	end = &timer_list->last_tl;
652 660
 	tl = timer_list->first_tl.next_tl;
653 661
 	while( tl!=end && tl->time_out <= time) {
654
-		tl->timer_list = NULL;
662
+		tl->timer_list = DETACHED_LIST;
655 663
 		tl=tl->next_tl;
656 664
 	}
657 665
 
... ...
@@ -682,7 +690,11 @@ struct timer_link  *check_and_split_time_list( struct timer *timer_list,
682 690
 
683 691
 
684 692
 
685
-/* stop timer */
693
+/* stop timer
694
+ * WARNING: a reset'ed timer will be lost forever
695
+ *  (succesive set_timer won't work unless you're lucky
696
+ *   an catch the race condition, the ideea here is there is no
697
+ *   guarantee you can do anything after a timer_reset)*/
686 698
 void reset_timer( struct timer_link* tl )
687 699
 {
688 700
 	/* disqualify this timer from execution by setting its time_out
... ...
@@ -700,7 +712,14 @@ void reset_timer( struct timer_link* tl )
700 712
 
701 713
 
702 714
 
703
-/* determine timer length and put on a correct timer list */
715
+/* determine timer length and put on a correct timer list
716
+ * WARNING: - don't try to use it to "move" a timer from one list
717
+ *            to another, you'll run into races
718
+ *          - reset_timer; set_timer might not work, a reset'ed timer
719
+ *             has no set_timer guarantee, it might be lost;
720
+ *             same for an expired timer: only it's handler can
721
+ *             set it again, an external set_timer has no guarantee
722
+ */
704 723
 void set_timer( struct timer_link *new_tl, enum lists list_id )
705 724
 {
706 725
 	unsigned int timeout;
... ...
@@ -718,9 +737,19 @@ void set_timer( struct timer_link *new_tl, enum lists list_id )
718 737
 	list= &(timertable->timers[ list_id ]);
719 738
 
720 739
 	lock(list->mutex);
740
+	/* check first if we are on the "detached" timer_routine list,
741
+	 * if so do nothing, the timer is not valid anymore
742
+	 * (sideffect: reset_timer ; set_timer is not safe, a reseted timer
743
+	 *  might be lost, depending on this race condition ) */
744
+	if (new_tl->timer_list==DETACHED_LIST){
745
+		LOG(L_CRIT, "WARNING: set_timer called on a \"detached\" timer"
746
+				" -- ignoring: %p\n", new_tl);
747
+		goto end;
748
+	}
721 749
 	/* make sure I'm not already on a list */
722 750
 	remove_timer_unsafe( new_tl );
723 751
 	add_timer_unsafe( list, new_tl, get_ticks()+timeout);
752
+end:
724 753
 	unlock(list->mutex);
725 754
 }
726 755
 
... ...
@@ -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