Browse Source

- stop retransmissions even if a final reply is received before we have
got a chance to start the retransmission timers.
Closes SER-362.

Andrei Pelinescu-Onciul 17 years ago
parent
commit
431de0d9c0
3 changed files with 24 additions and 1 deletions
  1. 1 0
      modules/tm/h_table.h
  2. 10 0
      modules/tm/timer.c
  3. 13 1
      modules/tm/timer.h

+ 1 - 0
modules/tm/h_table.h

@@ -142,6 +142,7 @@ enum kill_reason { REQ_FWDED=1, REQ_RPLD=2, REQ_RLSD=4, REQ_EXIST=8,
 #define F_RB_TIMEOUT	0x10 /* timeout */
 #define F_RB_REPLIED	0x20 /* reply received */
 #define F_RB_CANCELED	0x40 /* rb/branch canceled */
+#define F_RB_DEL_TIMER	0x80 /* timer should be deleted if active */
 
 
 /* if canceled or intended to be canceled, return true */

+ 10 - 0
modules/tm/timer.c

@@ -526,12 +526,22 @@ ticks_t retr_buf_handler(ticks_t ticks, struct timer_ln* tl, void *p)
 
 	rbuf=(struct  retr_buf*)
 			((void*)tl-(void*)(&((struct retr_buf*)0)->timer));
+	membar_depends(); /* to be on the safe side */
 	t=rbuf->my_T;
 	
 #ifdef TIMER_DEBUG
 	DBG("tm: timer retr_buf_handler @%d (%p -> %p -> %p)\n",
 			ticks, tl, rbuf, t);
 #endif
+	if (unlikely(rbuf->flags & F_RB_DEL_TIMER)){
+		/* timer marked for deletion */
+		rbuf->t_active=0; /* mark it as removed */
+		/* a membar is not really needed, in the very unlikely case that 
+		 * another process will see old t_active's value and will try to 
+		 * delete the timer again, but since timer_del it's safe in this cases
+		 * it will be a no-op */
+		return 0;
+	}
 	/* overflow safe check (should work ok for fr_intervals < max ticks_t/2) */
 	if ((s_ticks_t)(rbuf->fr_expire-ticks)<=0){
 		/* final response */

+ 13 - 1
modules/tm/timer.h

@@ -115,7 +115,7 @@ inline static int _set_fr_retr(struct retr_buf* rb, ticks_t retr)
 	eol=rb->my_T->end_of_life;
 	rb->timer.data=(void*)(unsigned long)(2*retr); /* hack , next retr. int. */
 	rb->retr_expire=ticks+retr;
-	if (rb->t_active){
+	if (unlikely(rb->t_active)){
 		/* we could have set_fr_retr called in the same time (acceptable 
 		 * race), we rely on timer_add adding it only once */
 #ifdef TIMER_DEBUG
@@ -138,6 +138,13 @@ inline static int _set_fr_retr(struct retr_buf* rb, ticks_t retr)
 		timeout=(((s_ticks_t)(eol-ticks))>0)?(eol-ticks):1; /* expire now */ 
 	}
 	atomic_cmpxchg_int((void*)&rb->fr_expire, 0, (int)(ticks+timeout));
+	if (unlikely(rb->flags & F_RB_DEL_TIMER)){
+		/* timer marked for deletion before we got a chance to add it
+		 * (e..g we got immediately a final reply before in another process)
+		 * => do nothing */
+		DBG("_set_fr_timer: too late, timer already marked for deletion\n");
+		return 0;
+	}
 #ifdef TIMER_DEBUG
 	ret=timer_add_safe(&(rb)->timer, (timeout<retr)?timeout:retr,
 							file, func, line);
@@ -145,6 +152,9 @@ inline static int _set_fr_retr(struct retr_buf* rb, ticks_t retr)
 	ret=timer_add(&(rb)->timer, (timeout<retr)?timeout:retr);
 #endif
 	if (ret==0) rb->t_active=1;
+	membar_write_atomic_op(); /* make sure t_active will be commited to mem.
+								 before the transaction would be deref. by the
+								 current process */
 	return ret;
 }
 
@@ -153,6 +163,8 @@ inline static int _set_fr_retr(struct retr_buf* rb, ticks_t retr)
 /* stop the timers assoc. with a retr. buf. */
 #define stop_rb_timers(rb) \
 do{ \
+	membar_depends(); \
+	(rb)->flags|=F_RB_DEL_TIMER; /* timer should be deleted */ \
 	if ((rb)->t_active){ \
 		(rb)->t_active=0; \
 		timer_del(&(rb)->timer); \