diff options
author | Benny Prijono <bennylp@teluu.com> | 2006-08-10 21:44:26 +0000 |
---|---|---|
committer | Benny Prijono <bennylp@teluu.com> | 2006-08-10 21:44:26 +0000 |
commit | d09368727007e87aaf50271297bfc1ce0d1a47a7 (patch) | |
tree | 5b719f1504b5b72f589e62d0c0a22ef3de04be04 /pjsip | |
parent | caad58f9b7daf65180a0614ba00d9d6145d7a007 (diff) |
Attempt to fix the race condition in dialog locking.
git-svn-id: http://svn.pjsip.org/repos/pjproject/trunk@671 74dad513-b988-da41-8d7b-12977e46ad98
Diffstat (limited to 'pjsip')
-rw-r--r-- | pjsip/include/pjsip/sip_dialog.h | 2 | ||||
-rw-r--r-- | pjsip/include/pjsip/sip_ua_layer.h | 19 | ||||
-rw-r--r-- | pjsip/src/pjsip-simple/evsub.c | 8 | ||||
-rw-r--r-- | pjsip/src/pjsip-ua/sip_inv.c | 33 | ||||
-rw-r--r-- | pjsip/src/pjsip/sip_dialog.c | 62 | ||||
-rw-r--r-- | pjsip/src/pjsip/sip_ua_layer.c | 63 |
6 files changed, 140 insertions, 47 deletions
diff --git a/pjsip/include/pjsip/sip_dialog.h b/pjsip/include/pjsip/sip_dialog.h index 123209be..191196a5 100644 --- a/pjsip/include/pjsip/sip_dialog.h +++ b/pjsip/include/pjsip/sip_dialog.h @@ -108,7 +108,7 @@ struct pjsip_dialog /* Dialog's system properties. */ char obj_name[PJ_MAX_OBJ_NAME]; /**< Standard id. */ pj_pool_t *pool; /**< Dialog's pool. */ - pj_mutex_t *mutex; /**< Dialog's mutex. Do not call!! + pj_mutex_t *mutex_; /**< Dialog's mutex. Do not call!! Use pjsip_dlg_inc_lock() instead! */ pjsip_user_agent *ua; /**< User agent instance. */ pjsip_endpoint *endpt; /**< Endpoint instance. */ diff --git a/pjsip/include/pjsip/sip_ua_layer.h b/pjsip/include/pjsip/sip_ua_layer.h index ca6db080..08e692ca 100644 --- a/pjsip/include/pjsip/sip_ua_layer.h +++ b/pjsip/include/pjsip/sip_ua_layer.h @@ -79,6 +79,25 @@ PJ_DECL(pj_status_t) pjsip_ua_init_module(pjsip_endpoint *endpt, */ PJ_DECL(pjsip_user_agent*) pjsip_ua_instance(void); + +/** + * Lock the dialog's hash table. This function is normally called by + * dialog code only. + * + * @return PJ_SUCCESS on success or the appropriate error code. + */ +PJ_DECL(pj_status_t) pjsip_ua_lock_dlg_table(void); + + +/** + * Unlock the dialog's hash table. This function is normally called by + * dialog code only. + * + * @return PJ_SUCCESS on success or the appropriate error code. + */ +PJ_DECL(pj_status_t) pjsip_ua_unlock_dlg_table(void); + + /** * Destroy the user agent layer. * diff --git a/pjsip/src/pjsip-simple/evsub.c b/pjsip/src/pjsip-simple/evsub.c index e5b4f526..a6f6b2ca 100644 --- a/pjsip/src/pjsip-simple/evsub.c +++ b/pjsip/src/pjsip-simple/evsub.c @@ -668,6 +668,9 @@ static pj_status_t evsub_create( pjsip_dialog *dlg, return PJSIP_SIMPLE_ENOPKG; + /* Must lock dialog before using pool etc. */ + pjsip_dlg_inc_lock(dlg); + /* Init attributes: */ sub = pj_pool_zalloc(dlg->pool, sizeof(struct pjsip_evsub)); @@ -714,8 +717,10 @@ static pj_status_t evsub_create( pjsip_dialog *dlg, /* Register as dialog usage: */ status = pjsip_dlg_add_usage(dlg, &mod_evsub.mod, dlgsub_head); - if (status != PJ_SUCCESS) + if (status != PJ_SUCCESS) { + pjsip_dlg_dec_lock(dlg); return status; + } PJ_LOG(5,(sub->obj_name, "%s subscription created, using dialog %s", @@ -723,6 +728,7 @@ static pj_status_t evsub_create( pjsip_dialog *dlg, dlg->obj_name)); *p_evsub = sub; + pjsip_dlg_dec_lock(dlg); return PJ_SUCCESS; } diff --git a/pjsip/src/pjsip-ua/sip_inv.c b/pjsip/src/pjsip-ua/sip_inv.c index 32947446..e363eb90 100644 --- a/pjsip/src/pjsip-ua/sip_inv.c +++ b/pjsip/src/pjsip-ua/sip_inv.c @@ -435,6 +435,9 @@ PJ_DEF(pj_status_t) pjsip_inv_create_uac( pjsip_dialog *dlg, /* Verify arguments. */ PJ_ASSERT_RETURN(dlg && p_inv, PJ_EINVAL); + /* Must lock dialog first */ + pjsip_dlg_inc_lock(dlg); + /* Normalize options */ if (options & PJSIP_INV_REQUIRE_100REL) options |= PJSIP_INV_SUPPORT_100REL; @@ -444,7 +447,7 @@ PJ_DEF(pj_status_t) pjsip_inv_create_uac( pjsip_dialog *dlg, /* Create the session */ inv = pj_pool_zalloc(dlg->pool, sizeof(pjsip_inv_session)); - PJ_ASSERT_RETURN(inv != NULL, PJ_ENOMEM); + pj_assert(inv != NULL); inv->pool = dlg->pool; inv->role = PJSIP_ROLE_UAC; @@ -461,14 +464,18 @@ PJ_DEF(pj_status_t) pjsip_inv_create_uac( pjsip_dialog *dlg, if (local_sdp) { status = pjmedia_sdp_neg_create_w_local_offer(dlg->pool, local_sdp, &inv->neg); - if (status != PJ_SUCCESS) + if (status != PJ_SUCCESS) { + pjsip_dlg_dec_lock(dlg); return status; + } } /* Register invite as dialog usage. */ status = pjsip_dlg_add_usage(dlg, &mod_inv.mod, inv); - if (status != PJ_SUCCESS) + if (status != PJ_SUCCESS) { + pjsip_dlg_dec_lock(dlg); return status; + } /* Increment dialog session */ pjsip_dlg_inc_session(dlg, &mod_inv.mod); @@ -476,6 +483,8 @@ PJ_DEF(pj_status_t) pjsip_inv_create_uac( pjsip_dialog *dlg, /* Done */ *p_inv = inv; + pjsip_dlg_dec_lock(dlg); + PJ_LOG(5,(inv->obj_name, "UAC invite session created for dialog %s", dlg->obj_name)); @@ -840,6 +849,9 @@ PJ_DEF(pj_status_t) pjsip_inv_create_uas( pjsip_dialog *dlg, msg->line.req.method.id == PJSIP_INVITE_METHOD, PJ_EINVALIDOP); + /* Lock dialog */ + pjsip_dlg_inc_lock(dlg); + /* Normalize options */ if (options & PJSIP_INV_REQUIRE_100REL) options |= PJSIP_INV_SUPPORT_100REL; @@ -849,7 +861,7 @@ PJ_DEF(pj_status_t) pjsip_inv_create_uas( pjsip_dialog *dlg, /* Create the session */ inv = pj_pool_zalloc(dlg->pool, sizeof(pjsip_inv_session)); - PJ_ASSERT_RETURN(inv != NULL, PJ_ENOMEM); + pj_assert(inv != NULL); inv->pool = dlg->pool; inv->role = PJSIP_ROLE_UAS; @@ -872,8 +884,10 @@ PJ_DEF(pj_status_t) pjsip_inv_create_uas( pjsip_dialog *dlg, if (status == PJ_SUCCESS) status = pjmedia_sdp_validate(rem_sdp); - if (status != PJ_SUCCESS) + if (status != PJ_SUCCESS) { + pjsip_dlg_dec_lock(dlg); return status; + } } /* Create negotiator. */ @@ -888,13 +902,17 @@ PJ_DEF(pj_status_t) pjsip_inv_create_uas( pjsip_dialog *dlg, status = PJ_SUCCESS; } - if (status != PJ_SUCCESS) + if (status != PJ_SUCCESS) { + pjsip_dlg_dec_lock(dlg); return status; + } /* Register invite as dialog usage. */ status = pjsip_dlg_add_usage(dlg, &mod_inv.mod, inv); - if (status != PJ_SUCCESS) + if (status != PJ_SUCCESS) { + pjsip_dlg_dec_lock(dlg); return status; + } /* Increment session in the dialog. */ pjsip_dlg_inc_session(dlg, &mod_inv.mod); @@ -909,6 +927,7 @@ PJ_DEF(pj_status_t) pjsip_inv_create_uas( pjsip_dialog *dlg, inv->invite_tsx->mod_data[mod_inv.mod.id] = tsx_inv_data; /* Done */ + pjsip_dlg_dec_lock(dlg); *p_inv = inv; PJ_LOG(5,(inv->obj_name, "UAS invite session created for dialog %s", diff --git a/pjsip/src/pjsip/sip_dialog.c b/pjsip/src/pjsip/sip_dialog.c index cc68ab12..3149be4c 100644 --- a/pjsip/src/pjsip/sip_dialog.c +++ b/pjsip/src/pjsip/sip_dialog.c @@ -80,7 +80,7 @@ static pj_status_t create_dialog( pjsip_user_agent *ua, pj_list_init(&dlg->inv_hdr); - status = pj_mutex_create_recursive(pool, "dlg%p", &dlg->mutex); + status = pj_mutex_create_recursive(pool, "dlg%p", &dlg->mutex_); if (status != PJ_SUCCESS) goto on_error; @@ -89,16 +89,18 @@ static pj_status_t create_dialog( pjsip_user_agent *ua, return PJ_SUCCESS; on_error: - if (dlg->mutex) - pj_mutex_destroy(dlg->mutex); + if (dlg->mutex_) + pj_mutex_destroy(dlg->mutex_); pjsip_endpt_release_pool(endpt, pool); return status; } static void destroy_dialog( pjsip_dialog *dlg ) { - if (dlg->mutex) - pj_mutex_destroy(dlg->mutex); + if (dlg->mutex_) { + pj_mutex_destroy(dlg->mutex_); + dlg->mutex_ = NULL; + } pjsip_endpt_release_pool(dlg->endpt, dlg->pool); } @@ -606,8 +608,7 @@ static pj_status_t unregister_and_destroy_dialog( pjsip_dialog *dlg ) PJ_LOG(5,(dlg->obj_name, "Dialog destroyed")); /* Destroy this dialog. */ - pj_mutex_destroy(dlg->mutex); - pjsip_endpt_release_pool(dlg->endpt, dlg->pool); + destroy_dialog(dlg); return PJ_SUCCESS; } @@ -638,13 +639,13 @@ PJ_DEF(pj_status_t) pjsip_dlg_set_route_set( pjsip_dialog *dlg, PJ_ASSERT_RETURN(dlg, PJ_EINVAL); - pj_mutex_lock(dlg->mutex); + pjsip_dlg_inc_lock(dlg); /* Clear route set. */ pj_list_init(&dlg->route_set); if (!route_set) { - pj_mutex_unlock(dlg->mutex); + pjsip_dlg_dec_lock(dlg); return PJ_SUCCESS; } @@ -658,8 +659,7 @@ PJ_DEF(pj_status_t) pjsip_dlg_set_route_set( pjsip_dialog *dlg, r = r->next; } - pj_mutex_unlock(dlg->mutex); - + pjsip_dlg_dec_lock(dlg); return PJ_SUCCESS; } @@ -672,9 +672,9 @@ PJ_DEF(pj_status_t) pjsip_dlg_inc_session( pjsip_dialog *dlg, { PJ_ASSERT_RETURN(dlg && mod, PJ_EINVAL); - pj_mutex_lock(dlg->mutex); + pjsip_dlg_inc_lock(dlg); ++dlg->sess_count; - pj_mutex_unlock(dlg->mutex); + pjsip_dlg_dec_lock(dlg); PJ_LOG(5,(dlg->obj_name, "Session count inc to %d by %.*s", dlg->sess_count, (int)mod->name.slen, mod->name.ptr)); @@ -684,12 +684,17 @@ PJ_DEF(pj_status_t) pjsip_dlg_inc_session( pjsip_dialog *dlg, /* * Lock dialog and increment session counter temporarily - * to prevent it from being deleted. + * to prevent it from being deleted. In addition, it must lock + * the user agent's dialog table first, to prevent deadlock. */ PJ_DEF(void) pjsip_dlg_inc_lock(pjsip_dialog *dlg) { - pj_mutex_lock(dlg->mutex); + pjsip_ua_lock_dlg_table(); + + pj_mutex_lock(dlg->mutex_); dlg->sess_count++; + + pjsip_ua_unlock_dlg_table(); } @@ -699,16 +704,20 @@ PJ_DEF(void) pjsip_dlg_inc_lock(pjsip_dialog *dlg) */ PJ_DEF(void) pjsip_dlg_dec_lock(pjsip_dialog *dlg) { + pjsip_ua_lock_dlg_table(); + pj_assert(dlg->sess_count > 0); --dlg->sess_count; if (dlg->sess_count==0 && dlg->tsx_count==0) { - pj_mutex_unlock(dlg->mutex); - pj_mutex_lock(dlg->mutex); + pj_mutex_unlock(dlg->mutex_); + pj_mutex_lock(dlg->mutex_); unregister_and_destroy_dialog(dlg); } else { - pj_mutex_unlock(dlg->mutex); + pj_mutex_unlock(dlg->mutex_); } + + pjsip_ua_unlock_dlg_table(); } @@ -724,7 +733,8 @@ PJ_DEF(pj_status_t) pjsip_dlg_dec_session( pjsip_dialog *dlg, PJ_LOG(5,(dlg->obj_name, "Session count dec to %d by %.*s", dlg->sess_count-1, (int)mod->name.slen, mod->name.ptr)); - pj_mutex_lock(dlg->mutex); + pjsip_dlg_inc_lock(dlg); + --dlg->sess_count; pjsip_dlg_dec_lock(dlg); return PJ_SUCCESS; @@ -748,7 +758,7 @@ PJ_DEF(pj_status_t) pjsip_dlg_add_usage( pjsip_dialog *dlg, "Module %.*s added as dialog usage, data=%p", (int)mod->name.slen, mod->name.ptr, mod_data)); - pj_mutex_lock(dlg->mutex); + pjsip_dlg_inc_lock(dlg); /* Usages are sorted on priority, lowest number first. * Find position to put the new module, also makes sure that @@ -757,7 +767,7 @@ PJ_DEF(pj_status_t) pjsip_dlg_add_usage( pjsip_dialog *dlg, for (index=0; index<dlg->usage_cnt; ++index) { if (dlg->usage[index] == mod) { pj_assert(!"This module is already registered"); - pj_mutex_unlock(dlg->mutex); + pjsip_dlg_dec_lock(dlg); return PJSIP_ETYPEEXISTS; } @@ -777,7 +787,7 @@ PJ_DEF(pj_status_t) pjsip_dlg_add_usage( pjsip_dialog *dlg, /* Increment count. */ ++dlg->usage_cnt; - pj_mutex_unlock(dlg->mutex); + pjsip_dlg_dec_lock(dlg); return PJ_SUCCESS; } @@ -896,7 +906,7 @@ PJ_DEF(pj_status_t) pjsip_dlg_create_request( pjsip_dialog *dlg, PJ_ASSERT_RETURN(dlg && method && p_tdata, PJ_EINVAL); /* Lock dialog. */ - pj_mutex_lock(dlg->mutex); + pjsip_dlg_inc_lock(dlg); /* Use outgoing CSeq and increment it by one. */ if (cseq <= 0) @@ -921,7 +931,7 @@ PJ_DEF(pj_status_t) pjsip_dlg_create_request( pjsip_dialog *dlg, } /* Unlock dialog. */ - pj_mutex_unlock(dlg->mutex); + pjsip_dlg_dec_lock(dlg); *p_tdata = tdata; @@ -1113,12 +1123,12 @@ PJ_DEF(pj_status_t) pjsip_dlg_create_response( pjsip_dialog *dlg, return status; /* Lock the dialog. */ - pj_mutex_lock(dlg->mutex); + pjsip_dlg_inc_lock(dlg); dlg_beautify_response(dlg, st_code, tdata); /* Unlock the dialog. */ - pj_mutex_unlock(dlg->mutex); + pjsip_dlg_dec_lock(dlg); /* Done. */ *p_tdata = tdata; diff --git a/pjsip/src/pjsip/sip_ua_layer.c b/pjsip/src/pjsip/sip_ua_layer.c index 2010dace..5e63e71c 100644 --- a/pjsip/src/pjsip/sip_ua_layer.c +++ b/pjsip/src/pjsip/sip_ua_layer.c @@ -209,6 +209,30 @@ PJ_DEF(pjsip_user_agent*) pjsip_ua_instance(void) } +/** + * Lock the dialog's hash table. This function is normally called by + * dialog code only. + * + * @return PJ_SUCCESS on success or the appropriate error code. + */ +PJ_DEF(pj_status_t) pjsip_ua_lock_dlg_table(void) +{ + return pj_mutex_lock(mod_ua.mutex); +} + + +/** + * Unlock the dialog's hash table. This function is normally called by + * dialog code only. + * + * @return PJ_SUCCESS on success or the appropriate error code. + */ +PJ_DEF(pj_status_t) pjsip_ua_unlock_dlg_table(void) +{ + return pj_mutex_unlock(mod_ua.mutex); +} + + /* * Get the endpoint where this UA is currently registered. */ @@ -561,12 +585,18 @@ static pj_bool_t mod_ua_on_rx_request(pjsip_rx_data *rdata) /* Mark the dialog id of the request. */ rdata->endpt_info.mod_data[mod_ua.mod.id] = dlg; + /* Lock the dialog */ + pjsip_dlg_inc_lock(dlg); + /* Done processing in the UA */ pj_mutex_unlock(mod_ua.mutex); /* Pass to dialog. */ pjsip_dlg_on_rx_request(dlg, rdata); + /* Unlock the dialog. This may destroy the dialog */ + pjsip_dlg_dec_lock(dlg); + /* Report as handled. */ return PJ_TRUE; } @@ -590,13 +620,19 @@ static pj_bool_t mod_ua_on_rx_response(pjsip_rx_data *rdata) * the response is a forked response. */ + /* Lock user agent dlg table before we're doing anything. */ + pj_mutex_lock(mod_ua.mutex); + /* Check if transaction is present. */ tsx = pjsip_rdata_get_tsx(rdata); if (tsx) { /* Check if dialog is present in the transaction. */ dlg = pjsip_tsx_get_dlg(tsx); - if (!dlg) + if (!dlg) { + /* Unlock dialog hash table. */ + pj_mutex_unlock(mod_ua.mutex); return PJ_FALSE; + } /* Get the dialog set. */ dlg_set = dlg->dlg_set; @@ -620,11 +656,11 @@ static pj_bool_t mod_ua_on_rx_response(pjsip_rx_data *rdata) * This must be some stateless response sent by other modules, * or a very late response. */ + /* Unlock dialog hash table. */ + pj_mutex_unlock(mod_ua.mutex); return PJ_FALSE; } - /* Lock user agent before accessing the hash table. */ - pj_mutex_lock(mod_ua.mutex); /* Get the dialog set. */ dlg_set = pj_hash_get(mod_ua.dlg_table, @@ -632,10 +668,10 @@ static pj_bool_t mod_ua_on_rx_response(pjsip_rx_data *rdata) rdata->msg_info.from->tag.slen, NULL); - /* Done with accessing the hash table. */ - pj_mutex_unlock(mod_ua.mutex); - if (!dlg_set) { + /* Unlock dialog hash table. */ + pj_mutex_unlock(mod_ua.mutex); + /* Strayed 2xx response!! */ PJ_LOG(4,(THIS_FILE, "Received strayed 2xx response (no dialog is found)" @@ -666,9 +702,6 @@ static pj_bool_t mod_ua_on_rx_response(pjsip_rx_data *rdata) int st_code = rdata->msg_info.msg->line.status.code; pj_str_t *to_tag = &rdata->msg_info.to->tag; - /* Must hold UA mutex before accessing dialog set. */ - pj_mutex_lock(mod_ua.mutex); - dlg = dlg_set->dlg_list.next; while (dlg != (pjsip_dialog*)&dlg_set->dlg_list) { @@ -733,9 +766,6 @@ static pj_bool_t mod_ua_on_rx_response(pjsip_rx_data *rdata) } - /* Done with the dialog set. */ - pj_mutex_unlock(mod_ua.mutex); - } else { /* Either this is a non-INVITE response, or subsequent INVITE * within dialog. The dialog should have been identified when @@ -751,9 +781,18 @@ static pj_bool_t mod_ua_on_rx_response(pjsip_rx_data *rdata) /* Put the dialog instance in the rdata. */ rdata->endpt_info.mod_data[mod_ua.mod.id] = dlg; + /* Acquire lock to the dialog. */ + pjsip_dlg_inc_lock(dlg); + + /* Unlock dialog hash table. */ + pj_mutex_unlock(mod_ua.mutex); + /* Pass the response to the dialog. */ pjsip_dlg_on_rx_response(dlg, rdata); + /* Unlock the dialog. This may destroy the dialog. */ + pjsip_dlg_dec_lock(dlg); + /* Done. */ return PJ_TRUE; } |