[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