[sr-dev] git:3.2: modules_k/pua: Fixed race hazard relating to RLS back-end SUBSCRIBEs

Module: sip-router Branch: 3.2 Commit: 047f66369f0c3eaa75592942f56565eb0f39f848 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=047f663… Author: pd &lt;peter.dunkley(a)crocodile-rcs.com&gt; Committer: pd &lt;peter.dunkley(a)crocodile-rcs.com&gt; Date: Fri Jan 27 15:34:16 2012 +0000 modules_k/pua: Fixed race hazard relating to RLS back-end SUBSCRIBEs - This resulted in the "no presence dialog record for non-TERMINATED state..." error message coming out of RLS a lot. - On the sending side you have can have two dialogs (one temporary and one full) stored for a short period of time. This is because the full dialog is written before the temporary one is deleted. This can make the lookup when a back-end NOTIFY is received fail because only one record is expected. This is now fixed - instead of inserting and then deleting we do a swap (while the hash table is locked). - Based on... (cherry picked from commit e627bc31776b521a1078b2a004e8ed179521cae2) --- modules_k/pua/hash.c | 48 ++++++++++++++++++++++++++------------- modules_k/pua/hash.h | 1 + modules_k/pua/send_subscribe.c | 16 ++++++++----- 3 files changed, 43 insertions(+), 22 deletions(-) diff --git a/modules_k/pua/hash.c b/modules_k/pua/hash.c index 805be74..1b6784c 100644 --- a/modules_k/pua/hash.c +++ b/modules_k/pua/hash.c @@ -215,33 +215,28 @@ void update_htable(ua_pres_t* p, time_t desired_expires, int expires, } } /* insert in front; so when searching the most recent result is returned*/ -void insert_htable(ua_pres_t* presentity) +void _insert_htable(ua_pres_t* presentity, unsigned int hash_code) { ua_pres_t* p= NULL; - unsigned int hash_code; - - hash_code= core_hash(presentity->pres_uri,presentity->watcher_uri, - HASH_SIZE); - - lock_get(&HashT->p_records[hash_code].lock); -/* - * useless since always checking before calling insert - if(get_dialog(presentity, hash_code)!= NULL ) - { - LM_DBG("Dialog already found- do not insert\n"); - return; - } -*/ p= HashT->p_records[hash_code].entity; presentity->db_flag= INSERTDB_FLAG; presentity->next= p->next; p->next= presentity; +} - lock_release(&HashT->p_records[hash_code].lock); +void insert_htable(ua_pres_t* presentity) +{ + unsigned int hash_code; + + hash_code= core_hash(presentity->pres_uri,presentity->watcher_uri, HASH_SIZE); + lock_get(&HashT->p_records[hash_code].lock); + + _insert_htable(presentity, hash_code); + lock_release(&HashT->p_records[hash_code].lock); } /* This function used to perform a search to find the hash table @@ -302,6 +297,27 @@ void destroy_htable(void) return; } +int convert_temporary_dialog(ua_pres_t *dialog) +{ + ua_pres_t *temp_dialog; + unsigned int hash_code; + + hash_code= core_hash(dialog->pres_uri,dialog->watcher_uri, HASH_SIZE); + lock_get(&HashT->p_records[hash_code].lock); + + temp_dialog = get_temporary_dialog(dialog, hash_code); + if (temp_dialog) + delete_htable(temp_dialog, hash_code); + else + return -1; + + _insert_htable(dialog, hash_code); + + lock_release(&HashT->p_records[hash_code].lock); + + return 1; +} + /* must lock the record line before calling this function*/ ua_pres_t* get_dialog(ua_pres_t* dialog, unsigned int hash_code) { diff --git a/modules_k/pua/hash.h b/modules_k/pua/hash.h index 141296f..c2204c4 100644 --- a/modules_k/pua/hash.h +++ b/modules_k/pua/hash.h @@ -126,6 +126,7 @@ int is_dialog(ua_pres_t* dialog); ua_pres_t* get_dialog(ua_pres_t* dialog, unsigned int hash_code); ua_pres_t* get_temporary_dialog(ua_pres_t* dialog, unsigned int hash_code); +int convert_temporary_dialog(ua_pres_t *dialog); int get_record_id(ua_pres_t* dialog, str** rec_id); typedef int (*get_record_id_t)(ua_pres_t* dialog, str** rec_id); diff --git a/modules_k/pua/send_subscribe.c b/modules_k/pua/send_subscribe.c index 08e3d3d..c4caad1 100644 --- a/modules_k/pua/send_subscribe.c +++ b/modules_k/pua/send_subscribe.c @@ -612,7 +612,11 @@ void subs_cback_func(struct cell *t, int cb_type, struct tmcb_params *ps) LM_DBG("record for subscribe from %.*s to %.*s inserted in datatbase\n", presentity->watcher_uri->len, presentity->watcher_uri->s, presentity->pres_uri->len, presentity->pres_uri->s); - insert_htable(presentity); + if (convert_temporary_dialog(presentity) < 0) + { + LM_ERR("Could not convert temporary dialog into a dialog\n"); + goto error; + } done: if(hentity->ua_flag == REQ_OTHER) @@ -620,13 +624,13 @@ done: hentity->flag= flag; run_pua_callbacks( hentity, msg); } + goto end; + error: - lock_get(&HashT->p_records[hash_code].lock); - presentity = get_temporary_dialog(hentity, hash_code); - if (presentity!=NULL) - delete_htable(presentity, hash_code); - lock_release(&HashT->p_records[hash_code].lock); + if (presentity->remote_contact.s) shm_free(presentity->remote_contact.s); + if (presentity) shm_free(presentity); +end: if(hentity) { shm_free(hentity);