[sr-dev] git:master: sca: fix race condition when two endpoints seize same index simultaneously

Andrew Mortensen admorten at isc.upenn.edu
Sun Mar 3 23:07:05 CET 2013


Module: sip-router
Branch: master
Commit: 522d06e75bf3c549af007701332f7db53a1b5ab6
URL:    http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=522d06e75bf3c549af007701332f7db53a1b5ab6

Author: Andrew Mortensen <admorten at isc.upenn.edu>
Committer: Andrew Mortensen <admorten at isc.upenn.edu>
Date:   Thu Feb 14 16:55:36 2013 -0500

sca: fix race condition when two endpoints seize same index simultaneously

- return 480 Temporarily Unavailable to loser of race.

---

 modules/sca/sca_appearance.c |   45 +++++++++++++++++++++++++++++++++-----
 modules/sca/sca_appearance.h |   10 ++++++--
 modules/sca/sca_call_info.c  |    2 +-
 modules/sca/sca_reply.c      |   14 +++++++++++-
 modules/sca/sca_subscribe.c  |   49 ++++++++++++++++++++++++++---------------
 5 files changed, 91 insertions(+), 29 deletions(-)

diff --git a/modules/sca/sca_appearance.c b/modules/sca/sca_appearance.c
index 81e304c..f613630 100644
--- a/modules/sca/sca_appearance.c
+++ b/modules/sca/sca_appearance.c
@@ -347,11 +347,12 @@ done:
 
     sca_appearance *
 sca_appearance_seize_index_unsafe( sca_mod *scam, str *aor, str *owner_uri,
-	int app_idx, int slot_idx )
+	int app_idx, int slot_idx, int *seize_error )
 {
     sca_appearance_list	*app_list;
     sca_appearance	*app = NULL;
     sca_hash_slot	*slot;
+    int			error = SCA_APPEARANCE_ERR_UNKNOWN;
 
     slot = sca_hash_table_slot_for_index( scam->appearances, slot_idx );
 
@@ -368,9 +369,8 @@ sca_appearance_seize_index_unsafe( sca_mod *scam, str *aor, str *owner_uri,
 	}
     }
     if ( app != NULL && app->index == app_idx ) {
-	LM_ERR( "sca_appearance_seize_index_unsafe: tried to seize in-use "
-		"%.*s appearance-index %d for %.*s", STR_FMT( aor ),
-		app_idx, STR_FMT( owner_uri ));
+	/* attempt to seize in-use appearance-index */
+	error = SCA_APPEARANCE_ERR_INDEX_UNAVAILABLE;
 	app = NULL;
 	goto done;
     }
@@ -379,16 +379,49 @@ sca_appearance_seize_index_unsafe( sca_mod *scam, str *aor, str *owner_uri,
     if ( app == NULL ) {
         LM_ERR( "Failed to create new appearance for %.*s at index %d",
                 STR_FMT( owner_uri ), app_idx );
+	error = SCA_APPEARANCE_ERR_MALLOC;
         goto done;
     }
     app->state = SCA_APPEARANCE_STATE_SEIZED;
 
     sca_appearance_list_insert_appearance( app_list, app );
 
+    error = SCA_APPEARANCE_OK;
+
 done:
+    if ( seize_error ) {
+	*seize_error = error;
+    }
+
     return( app );
 }
 
+    int
+sca_appearance_seize_index( sca_mod *scam, str *aor, int idx, str *owner_uri )
+{
+    sca_appearance	*app;
+    int			slot_idx;
+    int			app_idx = -1;
+    int			error = SCA_APPEARANCE_OK;
+
+    slot_idx = sca_hash_table_index_for_key( scam->appearances, aor );
+    sca_hash_table_lock_index( scam->appearances, slot_idx );
+
+    app = sca_appearance_seize_index_unsafe( scam, aor, owner_uri,
+						idx, slot_idx, &error );
+    if ( app != NULL ) {
+	app_idx = app->index;
+    }
+
+    sca_hash_table_unlock_index( scam->appearances, slot_idx );
+
+    if ( error == SCA_APPEARANCE_ERR_INDEX_UNAVAILABLE ) {
+	app_idx = SCA_APPEARANCE_INDEX_UNAVAILABLE;
+    }
+
+    return( app_idx );
+}
+
     sca_appearance *
 sca_appearance_seize_next_available_unsafe( sca_mod *scam, str *aor,
 	str *owner_uri, int slot_idx )
@@ -856,7 +889,7 @@ sca_appearance_update_index( sca_mod *scam, str *aor, int idx,
     if ( app == NULL ) {
 	LM_WARN( "Cannot update %.*s index %d to %.*s: index %d not in use",
 		 STR_FMT( aor ), idx, STR_FMT( &state_str ), idx );
-	rc = SCA_APPEARANCE_ERR_INVALID_INDEX;
+	rc = SCA_APPEARANCE_ERR_INDEX_INVALID;
 	goto done;
     }
 
@@ -974,7 +1007,7 @@ sca_appearance_release_index( sca_mod *scam, str *aor, int idx )
     if ( app == NULL ) {
 	LM_ERR( "Failed to unlink %.*s appearance-index %d: invalid index",
 		STR_FMT( aor ), idx );
-	rc = SCA_APPEARANCE_ERR_INVALID_INDEX;
+	rc = SCA_APPEARANCE_ERR_INDEX_INVALID;
 	goto done;
     }
     sca_appearance_free( app );
diff --git a/modules/sca/sca_appearance.h b/modules/sca/sca_appearance.h
index e1d623b..0d0abd8 100644
--- a/modules/sca/sca_appearance.h
+++ b/modules/sca/sca_appearance.h
@@ -48,10 +48,13 @@ enum {
 enum {
     SCA_APPEARANCE_OK = 0,
     SCA_APPEARANCE_ERR_NOT_IN_USE = 0x1001,
-    SCA_APPEARANCE_ERR_INVALID_INDEX = 0x1002,
-    SCA_APPEARANCE_ERR_MALLOC = 0x1004,
+    SCA_APPEARANCE_ERR_INDEX_INVALID = 0x1002,
+    SCA_APPEARANCE_ERR_INDEX_UNAVAILABLE = 0x1004,
+    SCA_APPEARANCE_ERR_MALLOC = 0x1008,
     SCA_APPEARANCE_ERR_UNKNOWN = 0x1f00,
 };
+#define SCA_APPEARANCE_INDEX_UNAVAILABLE	-2
+
 
 extern const str SCA_APPEARANCE_INDEX_STR;
 extern const str SCA_APPEARANCE_STATE_STR;
@@ -98,7 +101,8 @@ void	sca_appearance_state_to_str( int, str * );
 int	sca_appearance_state_from_str( str * );
 
 sca_appearance 	*sca_appearance_seize_index_unsafe( sca_mod *, str *, str *,
-								int, int );
+							int, int, int * );
+int	sca_appearance_seize_index( sca_mod *, str *, int, str * );
 int	sca_appearance_seize_next_available_index( sca_mod *, str *, str * );
 sca_appearance 	*sca_appearance_seize_next_available_unsafe( sca_mod *, str *,
 							     str *, int );
diff --git a/modules/sca/sca_call_info.c b/modules/sca/sca_call_info.c
index bf8db04..1a360ca 100644
--- a/modules/sca/sca_call_info.c
+++ b/modules/sca/sca_call_info.c
@@ -777,7 +777,7 @@ sca_call_info_uri_update( str *aor, sca_call_info *call_info,
 	rc = 1;
     } else {
 	app = sca_appearance_seize_index_unsafe( sca, aor, contact_uri,
-						call_info->index, slot_idx );
+					    call_info->index, slot_idx, NULL );
 	if ( app == NULL ) {
 	    LM_ERR( "sca_call_info_uri_update: failed to seize index %d "
 		    "for %.*s", call_info->index, STR_FMT( contact_uri ));
diff --git a/modules/sca/sca_reply.c b/modules/sca/sca_reply.c
index dfe66e4..8da3a6e 100644
--- a/modules/sca/sca_reply.c
+++ b/modules/sca/sca_reply.c
@@ -76,7 +76,19 @@ sca_reply( sca_mod *scam, int status_code, char *status_msg,
 	    LM_ERR( "Failed to add Allow-Events and Expires headers" );
 	    return( -1 );
 	}
-   }
+    } else if ( status_code == 480 ) {
+	/* tell loser of line-seize SUBSCRIBE race to try again shortly */ 
+	extra_headers.s = hdr_buf;
+	len = snprintf( extra_headers.s, sizeof( hdr_buf ),
+			"Retry-After: %d%s", 1, CRLF );
+	extra_headers.len = len;
+
+	if ( add_lump_rpl( msg, extra_headers.s, extra_headers.len,
+			    LUMP_RPL_HDR ) == NULL ) {
+	    LM_ERR( "sca_reply: failed to add Retry-After header" );
+	    return( -1 );
+	}
+    }
 
     if ( scam->sl_api->freply( msg, status_code, &status_str ) < 0 ) {
 	LM_ERR( "Failed to send \"%d %s\" reply to %.*s",
diff --git a/modules/sca/sca_subscribe.c b/modules/sca/sca_subscribe.c
index 36a4254..6337eba 100644
--- a/modules/sca/sca_subscribe.c
+++ b/modules/sca/sca_subscribe.c
@@ -1096,7 +1096,7 @@ sca_handle_subscribe( sip_msg_t *msg, char *p1, char *p2 )
     sca_subscription	req_sub;
     sca_subscription	*sub = NULL;
     sca_call_info	call_info;
-    hdr_field_t		*call_info_hdr;
+    hdr_field_t		*call_info_hdr = NULL;
     str			sub_key = STR_NULL;
     str			*to_tag = NULL;
     char		*status_text;
@@ -1151,6 +1151,23 @@ sca_handle_subscribe( sip_msg_t *msg, char *p1, char *p2 )
     /* pkg_malloc'd in sca_subscription_copy_subscription_key above */
     pkg_free( sub_key.s );
 
+    if ( req_sub.event == SCA_EVENT_TYPE_LINE_SEIZE ) {
+	call_info_hdr = sca_call_info_header_find( msg->headers );
+	if ( call_info_hdr ) {
+	    if ( sca_call_info_body_parse( &call_info_hdr->body,
+		    &call_info ) < 0 ) {
+		SCA_REPLY_ERROR( sca, 400, "Bad Request - "
+				"Invalid Call-Info header", msg );
+		goto done;
+	    }
+	    app_idx = call_info.index;
+	} else {
+	    SCA_REPLY_ERROR( sca, 400, "Bad Request - "
+			    "missing Call-Info header", msg );
+	    goto done;
+	}
+    }
+
     sca_hash_table_lock_index( sca->subscriptions, idx );
 
     sub = sca_hash_table_index_kv_find_unsafe( sca->subscriptions, idx,
@@ -1165,17 +1182,6 @@ sca_handle_subscribe( sip_msg_t *msg, char *p1, char *p2 )
 	}
 
 	if ( req_sub.event == SCA_EVENT_TYPE_LINE_SEIZE ) {
-	    call_info_hdr = sca_call_info_header_find( msg->headers );
-	    if ( call_info_hdr ) {
-		if ( sca_call_info_body_parse( &call_info_hdr->body,
-			&call_info ) < 0 ) {
-		    SCA_REPLY_ERROR( sca, 400, "Bad Request - "
-				    "Invalid Call-Info header", msg );
-		    goto done;
-		}
-		app_idx = call_info.index;
-	    }
-
 	    if ( req_sub.expires == 0 ) {
 		/* release the seized appearance */
 		if ( call_info_hdr == NULL ) {
@@ -1192,13 +1198,17 @@ sca_handle_subscribe( sip_msg_t *msg, char *p1, char *p2 )
 		}
 	    } else if ( SCA_STR_EMPTY( to_tag )) {
 		/* don't seize new index if this is a line-seize reSUBSCRIBE */
-		app_idx = sca_appearance_seize_next_available_index( sca,
-				&req_sub.target_aor, &req_sub.subscriber );
-		if ( app_idx < 0 ) {
+		app_idx = sca_appearance_seize_index( sca, &req_sub.target_aor,
+				app_idx, &req_sub.subscriber );
+		if ( app_idx == SCA_APPEARANCE_INDEX_UNAVAILABLE ) {
+		    SCA_REPLY_ERROR( sca, 480, "Temporarily Unavailable", msg );
+		    goto done;
+		} else if ( app_idx < 0 ) {
 		    SCA_REPLY_ERROR( sca, 500, "Internal Server Error - "
 					"seize appearance index", msg );
 		    goto done;
 		}
+		req_sub.index = app_idx;
 	    }
 	}
     } else {
@@ -1211,9 +1221,12 @@ sca_handle_subscribe( sip_msg_t *msg, char *p1, char *p2 )
 
 	if ( req_sub.expires > 0 ) {
 	    if ( req_sub.event == SCA_EVENT_TYPE_LINE_SEIZE ) {
-		app_idx = sca_appearance_seize_next_available_index( sca,
-				&req_sub.target_aor, &req_sub.subscriber );
-		if ( app_idx < 0 ) {
+		app_idx = sca_appearance_seize_index( sca, &req_sub.target_aor,
+				app_idx, &req_sub.subscriber );
+		if ( app_idx == SCA_APPEARANCE_INDEX_UNAVAILABLE ) {
+		    SCA_REPLY_ERROR( sca, 480, "Temporarily Unavailable", msg );
+		    goto done;
+		} else if ( app_idx < 0 ) {
 		    SCA_REPLY_ERROR( sca, 500, "Internal Server Error - "
 					"seize appearance index", msg );
 		    goto done;




More information about the sr-dev mailing list