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

Module: sip-router Branch: admorten/sca Commit: cebfc1599c665f0b678c0e6a86903dac6babb03f URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=cebfc15… Author: Andrew Mortensen &lt;admorten(a)isc.upenn.edu&gt; Committer: Andrew Mortensen &lt;admorten(a)isc.upenn.edu&gt; 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 d717eb1..ec0e2b6 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 2f7a7f0..e6d6ed6 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;