[asterisk-commits] file: branch file/pimp_sip_registration r383035 - /team/file/pimp_sip_registr...

[SVN commits to the Asterisk project]

Author: file
Date: Wed Mar 13 12:21:35 2013
New Revision: 383035

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=383035
Log:
Move destruction to the timer callback to fix a race condition between retrying and destroying.

Modified:
    team/file/pimp_sip_registration/res/res_sip_outbound_registration.c

Modified: team/file/pimp_sip_registration/res/res_sip_outbound_registration.c
URL: http://svnview.digium.com/svn/asterisk/team/file/pimp_sip_registration/res/res_sip_outbound_registration.c?view=diff&rev=383035&r1=383034&r2=383035
==============================================================================
--- team/file/pimp_sip_registration/res/res_sip_outbound_registration.c (original)
+++ team/file/pimp_sip_registration/res/res_sip_outbound_registration.c Wed Mar 13 12:21:35 2013
@@ -49,7 +49,7 @@
 	/*! \brief Outbound registration client */
 	pjsip_regc *client;
 	/*! \brief Timer entry for retrying on temporal responses */
-	pj_timer_entry timer;
+	pj_timer_entry *timer;
 	/*! \brief Current number of retries */
 	unsigned int retries;
 	/*! \brief Maximum number of retries permitted */
@@ -85,16 +85,22 @@
 	struct sip_outbound_registration_state *state;
 };
 
-/*! \brief Callback function for outbound registration attempt */
+/*! \brief Callback function for outbound registration attempt or destruction */
 static void sip_outbound_registration_attempt_cb(pj_timer_heap_t *timer_heap, struct pj_timer_entry *entry)
 {
 	pjsip_tx_data *tdata;
 
-	if (pjsip_regc_register(entry->user_data, PJ_TRUE, &tdata) != PJ_SUCCESS) {
-		return;
-	}
-
-	pjsip_regc_send(entry->user_data, tdata);
+	if (entry->id == 1) {
+		if (pjsip_regc_register(entry->user_data, PJ_TRUE, &tdata) != PJ_SUCCESS) {
+			return;
+		}
+		pjsip_regc_send(entry->user_data, tdata);
+	} else if (entry->id == 2) {
+		if (pjsip_regc_unregister(entry->user_data, &tdata) == PJ_SUCCESS) {
+			pjsip_regc_send(entry->user_data, tdata);
+		}
+		pjsip_regc_destroy(entry->user_data);
+	}
 }
 
 /* \brief Helper funtion which determines if a response code is temporal or not */
@@ -131,13 +137,13 @@
 		if (state->retries == state->max_retries) {
 			state->status = SIP_REGISTRATION_REJECTED_PERMANENT;
 			ast_log(LOG_WARNING, "Maximum retries reached when attempting outbound registration to '%s' with client '%s', stopping registration attempt\n",
-				server_uri, client_uri);;
+				server_uri, client_uri);
 		} else {
 			pj_time_val delay = { .sec = state->retry_interval, };
 
 			state->status = SIP_REGISTRATION_REJECTED_TEMPORARY;
 			state->retries++;
-			pjsip_endpt_schedule_timer(ast_sip_get_pjsip_endpoint(), &state->timer, &delay);
+			pjsip_endpt_schedule_timer(ast_sip_get_pjsip_endpoint(), state->timer, &delay);
 			ast_log(LOG_WARNING, "Temporal response '%d' received from '%s' on registration attempt to '%s', retrying in '%d' seconds\n",
 				param->code, server_uri, client_uri, state->retry_interval);
 		}
@@ -152,23 +158,19 @@
 static void sip_outbound_registration_state_destroy(void *obj)
 {
 	struct sip_outbound_registration_state *state = obj;
+	pj_time_val delay = { .sec = 1, };
 
 	if (!state->client) {
 		return;
 	}
 
-	pjsip_endpt_cancel_timer(ast_sip_get_pjsip_endpoint(), &state->timer);
-
-	/* If we have at least attempted registering once send an unregister */
-	if (state->timer.id) {
-		pjsip_tx_data *tdata;
-
-		if (pjsip_regc_unregister(state->client, &tdata) == PJ_SUCCESS) {
-			pjsip_regc_send(state->client, tdata);
-		}
-	}
-
-	pjsip_regc_destroy(state->client);
+	if (state->status != SIP_REGISTRATION_UNREGISTERED) {
+		pjsip_endpt_cancel_timer(ast_sip_get_pjsip_endpoint(), state->timer);
+		state->timer->id = 2;
+		pjsip_endpt_schedule_timer(ast_sip_get_pjsip_endpoint(), state->timer, &delay);
+	} else {
+		pjsip_regc_destroy(state->client);
+	}
 }
 
 /*! \brief Allocator function for registration state */
@@ -178,15 +180,16 @@
 
 	if (!state) {
 		return NULL;
-	} else if ((pjsip_regc_create(ast_sip_get_pjsip_endpoint(), state, sip_outbound_registration_response_cb, &state->client) != PJ_SUCCESS)) {
+	} else if ((pjsip_regc_create(ast_sip_get_pjsip_endpoint(), state, sip_outbound_registration_response_cb, &state->client) != PJ_SUCCESS) ||
+		!(state->timer = PJ_POOL_ZALLOC_T(pjsip_regc_get_pool(state->client), struct pj_timer_entry))) {
 		ao2_cleanup(state);
 		return NULL;
 	}
 
 	state->status = SIP_REGISTRATION_UNREGISTERED;
-	state->timer.id = 0;
-	state->timer.user_data = state->client;
-	state->timer.cb = sip_outbound_registration_attempt_cb;
+	state->timer->id = 1;
+	state->timer->user_data = state->client;
+	state->timer->cb = sip_outbound_registration_attempt_cb;
 
 	return state;
 }
@@ -363,10 +366,8 @@
 	pjsip_regc_update_expires(registration->state->client, registration->expiration);
 
 	/* Cancel retry attept if present and try a new registration */
-	pjsip_endpt_cancel_timer(ast_sip_get_pjsip_endpoint(), &registration->state->timer);
-
-	registration->state->timer.id = 1;
-	pjsip_endpt_schedule_timer(ast_sip_get_pjsip_endpoint(), &registration->state->timer, &delay);
+	pjsip_endpt_cancel_timer(ast_sip_get_pjsip_endpoint(), registration->state->timer);
+	pjsip_endpt_schedule_timer(ast_sip_get_pjsip_endpoint(), registration->state->timer, &delay);
 
 	return 0;
 }