From 208181b3db6080dfca9e0b98456d4746cfc346fa Mon Sep 17 00:00:00 2001 From: Nanang Izzuddin Date: Wed, 24 Jul 2013 08:06:59 +0000 Subject: Fix #1691: Apply group lock mechanism in NAT detect to avoid deadlock. git-svn-id: http://svn.pjsip.org/repos/pjproject/trunk@4573 74dad513-b988-da41-8d7b-12977e46ad98 --- pjnath/src/pjnath/nat_detect.c | 59 +++++++++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 21 deletions(-) (limited to 'pjnath/src') diff --git a/pjnath/src/pjnath/nat_detect.c b/pjnath/src/pjnath/nat_detect.c index 546f63d4..a4fa588e 100644 --- a/pjnath/src/pjnath/nat_detect.c +++ b/pjnath/src/pjnath/nat_detect.c @@ -75,7 +75,7 @@ enum timer_type typedef struct nat_detect_session { pj_pool_t *pool; - pj_mutex_t *mutex; + pj_grp_lock_t *grp_lock; pj_timer_heap_t *timer_heap; pj_timer_entry timer; @@ -133,7 +133,7 @@ static pj_status_t send_test(nat_detect_session *sess, static void on_sess_timer(pj_timer_heap_t *th, pj_timer_entry *te); static void sess_destroy(nat_detect_session *sess); - +static void sess_on_destroy(void *member); /* * Get the NAT name from the specified NAT type. @@ -233,10 +233,16 @@ PJ_DEF(pj_status_t) pj_stun_detect_nat_type(const pj_sockaddr_in *server, sess->user_data = user_data; sess->cb = cb; - status = pj_mutex_create_recursive(pool, pool->obj_name, &sess->mutex); - if (status != PJ_SUCCESS) - goto on_error; - + status = pj_grp_lock_create(pool, NULL, &sess->grp_lock); + if (status != PJ_SUCCESS) { + /* Group lock not created yet, just destroy pool and return */ + pj_pool_release(pool); + return status; + } + + pj_grp_lock_add_ref(sess->grp_lock); + pj_grp_lock_add_handler(sess->grp_lock, pool, sess, &sess_on_destroy); + pj_memcpy(&sess->server, server, sizeof(pj_sockaddr_in)); /* @@ -294,9 +300,9 @@ PJ_DEF(pj_status_t) pj_stun_detect_nat_type(const pj_sockaddr_in *server, pj_bzero(&ioqueue_cb, sizeof(ioqueue_cb)); ioqueue_cb.on_read_complete = &on_read_complete; - status = pj_ioqueue_register_sock(sess->pool, stun_cfg->ioqueue, - sess->sock, sess, &ioqueue_cb, - &sess->key); + status = pj_ioqueue_register_sock2(sess->pool, stun_cfg->ioqueue, + sess->sock, sess->grp_lock, sess, + &ioqueue_cb, &sess->key); if (status != PJ_SUCCESS) goto on_error; @@ -307,7 +313,7 @@ PJ_DEF(pj_status_t) pj_stun_detect_nat_type(const pj_sockaddr_in *server, sess_cb.on_request_complete = &on_request_complete; sess_cb.on_send_msg = &on_send_msg; status = pj_stun_session_create(stun_cfg, pool->obj_name, &sess_cb, - PJ_FALSE, NULL, &sess->stun_sess); + PJ_FALSE, sess->grp_lock, &sess->stun_sess); if (status != PJ_SUCCESS) goto on_error; @@ -338,24 +344,31 @@ static void sess_destroy(nat_detect_session *sess) { if (sess->stun_sess) { pj_stun_session_destroy(sess->stun_sess); + sess->stun_sess = NULL; } if (sess->key) { pj_ioqueue_unregister(sess->key); + sess->key = NULL; + sess->sock = PJ_INVALID_SOCKET; } else if (sess->sock && sess->sock != PJ_INVALID_SOCKET) { pj_sock_close(sess->sock); + sess->sock = PJ_INVALID_SOCKET; } - if (sess->mutex) { - pj_mutex_destroy(sess->mutex); + if (sess->grp_lock) { + pj_grp_lock_dec_ref(sess->grp_lock); } +} +static void sess_on_destroy(void *member) +{ + nat_detect_session *sess = (nat_detect_session*)member; if (sess->pool) { pj_pool_release(sess->pool); } } - static void end_session(nat_detect_session *sess, pj_status_t status, pj_stun_nat_type nat_type) @@ -402,7 +415,11 @@ static void on_read_complete(pj_ioqueue_key_t *key, sess = (nat_detect_session *) pj_ioqueue_get_user_data(key); pj_assert(sess != NULL); - pj_mutex_lock(sess->mutex); + pj_grp_lock_acquire(sess->grp_lock); + + /* Ignore packet when STUN session has been destroyed */ + if (!sess->stun_sess) + goto on_return; if (bytes_read < 0) { if (-bytes_read != PJ_STATUS_FROM_OS(OSERR_EWOULDBLOCK) && @@ -435,7 +452,7 @@ static void on_read_complete(pj_ioqueue_key_t *key, } on_return: - pj_mutex_unlock(sess->mutex); + pj_grp_lock_release(sess->grp_lock); } @@ -490,7 +507,7 @@ static void on_request_complete(pj_stun_session *stun_sess, sess = (nat_detect_session*) pj_stun_session_get_user_data(stun_sess); - pj_mutex_lock(sess->mutex); + pj_grp_lock_acquire(sess->grp_lock); /* Find errors in the response */ if (status == PJ_SUCCESS) { @@ -786,7 +803,7 @@ static void on_request_complete(pj_stun_session *stun_sess, } on_return: - pj_mutex_unlock(sess->mutex); + pj_grp_lock_release(sess->grp_lock); } @@ -865,12 +882,12 @@ static void on_sess_timer(pj_timer_heap_t *th, sess = (nat_detect_session*) te->user_data; if (te->id == TIMER_DESTROY) { - pj_mutex_lock(sess->mutex); + pj_grp_lock_acquire(sess->grp_lock); pj_ioqueue_unregister(sess->key); sess->key = NULL; sess->sock = PJ_INVALID_SOCKET; te->id = 0; - pj_mutex_unlock(sess->mutex); + pj_grp_lock_release(sess->grp_lock); sess_destroy(sess); @@ -878,7 +895,7 @@ static void on_sess_timer(pj_timer_heap_t *th, pj_bool_t next_timer; - pj_mutex_lock(sess->mutex); + pj_grp_lock_acquire(sess->grp_lock); next_timer = PJ_FALSE; @@ -903,7 +920,7 @@ static void on_sess_timer(pj_timer_heap_t *th, te->id = 0; } - pj_mutex_unlock(sess->mutex); + pj_grp_lock_release(sess->grp_lock); } else { pj_assert(!"Invalid timer ID"); -- cgit v1.2.3