From 9d807d1d8af24c0163d3bb2ae876a87a8b637494 Mon Sep 17 00:00:00 2001 From: Daniel-Constantin Mierla Date: Thu, 1 Oct 2015 15:49:26 +0200 Subject: [PATCH] dialog: keep slot locked when searching for duplicate dialog - when attempting to create a new dialog, the function searching to see if it is already one with same attributes keeps the slot locked so is no race in between the return of function and building the new dlg structure - if the dlg is found, release the lock after figuring out it is a spiral or not --- modules/dialog/dlg_handlers.c | 24 +++++++++--------------- modules/dialog/dlg_hash.c | 8 ++++---- modules/dialog/dlg_hash.h | 6 +++--- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/modules/dialog/dlg_handlers.c b/modules/dialog/dlg_handlers.c index 1ba3b5ada64..3117e331fe9 100644 --- a/modules/dialog/dlg_handlers.c +++ b/modules/dialog/dlg_handlers.c @@ -766,7 +766,6 @@ int dlg_new_dialog(sip_msg_t *req, struct cell *t, const int run_initial_cbs) str ttag; str req_uri; unsigned int dir; - int mlock; dlg = dlg_get_ctx_dialog(); if(dlg != NULL) { @@ -792,14 +791,10 @@ int dlg_new_dialog(sip_msg_t *req, struct cell *t, const int run_initial_cbs) trim(&req_uri); dir = DLG_DIR_NONE; - mlock = 1; /* search dialog by SIP attributes - * - if not found, hash table slot is left locked, to avoid races - * to add 'same' dialog on parallel forking or not-handled-yet - * retransmissions. Release slot after linking new dialog */ - dlg = search_dlg(&callid, &ftag, &ttag, &dir); + * - hash table slot is left locked */ + dlg = dlg_search(&callid, &ftag, &ttag, &dir); if(dlg) { - mlock = 0; if (detect_spirals) { if (spiral_detected == 1) return 0; @@ -817,13 +812,11 @@ int dlg_new_dialog(sip_msg_t *req, struct cell *t, const int run_initial_cbs) _dlg_ctx.iuid.h_id = dlg->h_id; /* search_dlg() has incremented the ref count by 1 */ dlg_release(dlg); + dlg_hash_release(&callid); return 0; } dlg_release(dlg); } - /* lock the slot - dlg found, but in dlg_state_deleted, do a new one */ - dlg_hash_lock(&callid); - mlock = 1; } spiral_detected = 0; @@ -834,7 +827,7 @@ int dlg_new_dialog(sip_msg_t *req, struct cell *t, const int run_initial_cbs) &req_uri /*r-uri*/ ); if (dlg==0) { - if(likely(mlock==1)) dlg_hash_release(&callid); + dlg_hash_release(&callid); LM_ERR("failed to create new dialog\n"); return -1; } @@ -842,7 +835,7 @@ int dlg_new_dialog(sip_msg_t *req, struct cell *t, const int run_initial_cbs) /* save caller's tag, cseq, contact and record route*/ if (populate_leg_info(dlg, req, t, DLG_CALLER_LEG, &(get_from(req)->tag_value)) !=0) { - if(likely(mlock==1)) dlg_hash_release(&callid); + dlg_hash_release(&callid); LM_ERR("could not add further info to the dialog\n"); shm_free(dlg); return -1; @@ -851,9 +844,10 @@ int dlg_new_dialog(sip_msg_t *req, struct cell *t, const int run_initial_cbs) /* Populate initial varlist: */ dlg->vars = get_local_varlist_pointer(req, 1); - /* if search_dlg() returned NULL, slot was kept locked */ - link_dlg(dlg, 0, mlock); - if(likely(mlock==1)) dlg_hash_release(&callid); + /* after dlg_search() slot was kept locked */ + link_dlg(dlg, 0, 0); + /* unlock after dlg_search() */ + dlg_hash_release(&callid); dlg->lifetime = get_dlg_timeout(req); s.s = _dlg_ctx.to_route_name; diff --git a/modules/dialog/dlg_hash.c b/modules/dialog/dlg_hash.c index 75a0ed7c261..d20a2151adb 100644 --- a/modules/dialog/dlg_hash.c +++ b/modules/dialog/dlg_hash.c @@ -738,7 +738,7 @@ static inline struct dlg_cell* internal_get_dlg(unsigned int h_entry, /* Check callid / fromtag / totag */ if (match_dialog( dlg, callid, ftag, ttag, dir)==1) { ref_dlg_unsafe(dlg, 1); - dlg_unlock( d_table, d_entry); + if(likely(mode==0)) dlg_unlock( d_table, d_entry); LM_DBG("dialog callid='%.*s' found on entry %u, dir=%d\n", callid->len, callid->s,h_entry,*dir); return dlg; @@ -794,15 +794,15 @@ struct dlg_cell* get_dlg( str *callid, str *ftag, str *ttag, unsigned int *dir) * referred to as a dialog." * Note that the caller is responsible for decrementing (or reusing) * the reference counter by one again if a dialog has been found. - * If the dialog is not found, the hash slot is left locked, to allow - * linking the structure of a new dialog. + * Important: the hash slot is left locked (e.g., needed to allow + * linking the structure of a new dialog). * \param callid callid * \param ftag from tag * \param ttag to tag * \param dir direction * \return dialog structure on success, NULL on failure (and slot locked) */ -dlg_cell_t* search_dlg( str *callid, str *ftag, str *ttag, unsigned int *dir) +dlg_cell_t* dlg_search( str *callid, str *ftag, str *ttag, unsigned int *dir) { struct dlg_cell *dlg; unsigned int he; diff --git a/modules/dialog/dlg_hash.h b/modules/dialog/dlg_hash.h index 6b9e941aa32..166174ecf30 100644 --- a/modules/dialog/dlg_hash.h +++ b/modules/dialog/dlg_hash.h @@ -345,15 +345,15 @@ dlg_cell_t* get_dlg(str *callid, str *ftag, str *ttag, unsigned int *dir); * referred to as a dialog." * Note that the caller is responsible for decrementing (or reusing) * the reference counter by one again if a dialog has been found. - * If the dialog is not found, the hash slot is left locked, to allow - * linking the structure of a new dialog. + * Important: the hash slot is left locked (e.g., needed to allow + * linking the structure of a new dialog). * \param callid callid * \param ftag from tag * \param ttag to tag * \param dir direction * \return dialog structure on success, NULL on failure (and slot locked) */ -dlg_cell_t* search_dlg(str *callid, str *ftag, str *ttag, unsigned int *dir); +dlg_cell_t* dlg_search(str *callid, str *ftag, str *ttag, unsigned int *dir); /*!