retransmit response to BYE requests until timer J expires

According to RFC 3261 section 17.2.2, which describes non-INVITE server
transaction, when a dialog enters the Completed state it must destroy
the dialog after Timer J (T1*64) fires.  For a BYE transaction Asterisk
terminates the dialog immediately during sip_hangup() when it should be
waiting T1*64 ms.  This results in some odd behavior.  For instance if
Asterisk receives a BYE and transmits a 200ok in response, if the endpoint
never receives the 200ok it will retransmit the BYE to which Asterisk
responds with a "481 Call leg/transaction does not exist" because the
dialog is already gone.

To resolve this I made a function called sip_scheddestroy_final().  This
differs slightly from sip_schedestroy() in that it enables a flag that
will prevent the destruction from ever being rescheduled or canceled
afterwards.  It also prevents the pvt's needdestroy flag from being set
which triggers the destruction of the dialog within the do_monitor thread().
By using this function we are guaranteed destruction will not occur
until the scheduled time.  This allows Asterisk to respond to any possible
retransmits for a dialog after we process the initial BYE request for T1*64 ms.

Other changes: I removed two instances where sip_cancel_destroy is used
right before calling sip_scheddestroy.  sip_scheddestroy always calls
sip_cancel_destroy before scheduling the new destruction so it is completely
unnecessary.

Review: https://reviewboard.asterisk.org/r/694/



git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@271262 65c4cc65-6c06-0410-ace0-fbb531ad65f3

3 files changed, 34 insertions, 7 deletions

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 32f232c62..5e9d2d26e 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -2816,6 +2816,9 @@ static const char *referstatus2str(enum referstatus rstatus)
 
 static inline void pvt_set_needdestroy(struct sip_pvt *pvt, const char *reason)
 {
+	if (pvt->final_destruction_scheduled) {
+		return; /* This is already scheduled for final destruction, let the scheduler take care of it. */
+	}
 	append_history(pvt, "NeedDestroy", "Setting needdestroy because %s", reason);
 	pvt->needdestroy = 1;
 }
@@ -3564,9 +3567,28 @@ static int __sip_autodestruct(const void *data)
 	return 0;
 }
 
+/*! \brief Schedule final destruction of SIP dialog.  This can not be canceled.
+ *  This function is used to keep a dialog around for a period of time in order
+ *  to properly respond to any retransmits. */
+void sip_scheddestroy_final(struct sip_pvt *p, int ms)
+{
+	if (p->final_destruction_scheduled) {
+		return; /* already set final destruction */
+	}
+
+	sip_scheddestroy(p, ms);
+	if (p->autokillid != -1) {
+		p->final_destruction_scheduled = 1;
+	}
+}
+
 /*! \brief Schedule destruction of SIP dialog */
 void sip_scheddestroy(struct sip_pvt *p, int ms)
 {
+	if (p->final_destruction_scheduled) {
+		return; /* already set final destruction */
+	}
+
 	if (ms < 0) {
 		if (p->timer_t1 == 0) {
 			p->timer_t1 = global_t1;	/* Set timer T1 if not set (RFC 3261) */
@@ -3596,6 +3618,11 @@ void sip_scheddestroy(struct sip_pvt *p, int ms)
 int sip_cancel_destroy(struct sip_pvt *p)
 {
 	int res = 0;
+
+	if (p->final_destruction_scheduled) {
+		return res;
+	}
+
 	if (p->autokillid > -1) {
 		int res3;
 
@@ -5675,8 +5702,6 @@ static int sip_hangup(struct ast_channel *ast)
 			update_call_counter(p, DEC_CALL_LIMIT);
 		}
 		ast_debug(4, "SIP Transfer: Not hanging up right now... Rescheduling hangup for %s.\n", p->callid);
-		if (p->autokillid > -1 && sip_cancel_destroy(p))
-			ast_log(LOG_WARNING, "Unable to cancel SIP destruction.  Expect bad things.\n");
 		sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
 		ast_clear_flag(&p->flags[0], SIP_DEFER_BYE_ON_TRANSFER);	/* Really hang up next time */
 		p->needdestroy = 0;
@@ -13006,8 +13031,6 @@ static int cb_extensionstate(char *context, char* exten, int state, void *data)
 	switch(state) {
 	case AST_EXTENSION_DEACTIVATED:	/* Retry after a while */
 	case AST_EXTENSION_REMOVED:	/* Extension is gone */
-		if (p->autokillid > -1 && sip_cancel_destroy(p))	/* Remove subscription expiry for renewals */
-			ast_log(LOG_WARNING, "Unable to cancel SIP destruction.  Expect bad things.\n");
 		sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);	/* Delete subscription in 32 secs */
 		ast_verb(2, "Extension state: Watcher for hint %s %s. Notify User %s\n", exten, state == AST_EXTENSION_DEACTIVATED ? "deactivated" : "removed", p->username);
 		p->stateid = -1;
@@ -21976,9 +21999,10 @@ static int handle_request_bye(struct sip_pvt *p, struct sip_request *req)
 	} else if (p->owner) {
 		ast_set_hangupsource(p->owner, p->owner->name, 0);
 		ast_queue_hangup(p->owner);
+		sip_scheddestroy_final(p, DEFAULT_TRANS_TIMEOUT);
 		ast_debug(3, "Received bye, issuing owner hangup\n");
 	} else {
-		sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
+		sip_scheddestroy_final(p, DEFAULT_TRANS_TIMEOUT);
 		ast_debug(3, "Received bye, no owner, selfdestruct soon.\n");
 	}
 	ast_clear_flag(&p->flags[1], SIP_PAGE2_DIALOG_ESTABLISHED);
diff --git a/channels/sip/include/dialog.h b/channels/sip/include/dialog.h
index 554aa5e6a..8972c02d9 100644
--- a/channels/sip/include/dialog.h
+++ b/channels/sip/include/dialog.h
@@ -36,6 +36,7 @@ struct sip_pvt *dialog_unref_debug(struct sip_pvt *p, char *tag, char *file, int
 
 struct sip_pvt *sip_alloc(ast_string_field callid, struct sockaddr_in *sin,
 				 int useglobal_nat, const int intended_method, struct sip_request *req);
+void sip_scheddestroy_final(struct sip_pvt *p, int ms);
 void sip_scheddestroy(struct sip_pvt *p, int ms);
 int sip_cancel_destroy(struct sip_pvt *p);
 
diff --git a/channels/sip/include/sip.h b/channels/sip/include/sip.h
index 8d6d0abcb..95ad34e31 100644
--- a/channels/sip/include/sip.h
+++ b/channels/sip/include/sip.h
@@ -952,8 +952,10 @@ struct sip_pvt {
 
 	/* boolean flags that don't belong in flags */
 	unsigned short do_history:1;          /*!< Set if we want to record history */
-	unsigned short alreadygone:1;         /*!< already destroyed by our peer */
-	unsigned short needdestroy:1;         /*!< need to be destroyed by the monitor thread */
+	unsigned short alreadygone:1;         /*!< the peer has sent a message indicating termination of the dialog */
+	unsigned short needdestroy:1;         /*!< this dialog needs to be destroyed by the monitor thread */
+	unsigned short final_destruction_scheduled:1; /*!< final dialog destruction is scheduled. Keep dialog
+	                                               *   around until then to handle retransmits. */
 	unsigned short outgoing_call:1;       /*!< this is an outgoing call */
 	unsigned short answered_elsewhere:1;  /*!< This call is cancelled due to answer on another channel */
 	unsigned short novideo:1;             /*!< Didn't get video in invite, don't offer */