mirror of https://github.com/asterisk/asterisk
Joshua Colp
8 years ago
committed by
Gerrit Code Review
commit
0b00df55fc
@ -0,0 +1,32 @@
|
|||||||
|
Index: trunk/pjsip/src/pjsip/sip_transaction.c
|
||||||
|
===================================================================
|
||||||
|
--- a/pjsip/src/pjsip/sip_transaction.c (revision 5244)
|
||||||
|
+++ b/pjsip/src/pjsip/sip_transaction.c (revision 5572)
|
||||||
|
@@ -1231,5 +1231,27 @@
|
||||||
|
PJSIP_EVENT_INIT_TSX_STATE(e, tsx, event_src_type, event_src,
|
||||||
|
prev_state);
|
||||||
|
+
|
||||||
|
+ /* For timer event, release lock to avoid deadlock.
|
||||||
|
+ * This should be safe because:
|
||||||
|
+ * 1. The tsx state just switches to TERMINATED or DESTROYED.
|
||||||
|
+ * 2. There should be no other processing taking place. All other
|
||||||
|
+ * events, such as the ones handled by tsx_on_state_terminated()
|
||||||
|
+ * should be ignored.
|
||||||
|
+ * 3. tsx_shutdown() hasn't been called.
|
||||||
|
+ * Refer to ticket #2001 (https://trac.pjsip.org/repos/ticket/2001).
|
||||||
|
+ */
|
||||||
|
+ if (event_src_type == PJSIP_EVENT_TIMER &&
|
||||||
|
+ (pj_timer_entry *)event_src == &tsx->timeout_timer)
|
||||||
|
+ {
|
||||||
|
+ pj_grp_lock_release(tsx->grp_lock);
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
(*tsx->tsx_user->on_tsx_state)(tsx, &e);
|
||||||
|
+
|
||||||
|
+ if (event_src_type == PJSIP_EVENT_TIMER &&
|
||||||
|
+ (pj_timer_entry *)event_src == &tsx->timeout_timer)
|
||||||
|
+ {
|
||||||
|
+ pj_grp_lock_acquire(tsx->grp_lock);
|
||||||
|
+ }
|
||||||
|
}
|
||||||
|
|
||||||
@ -0,0 +1,119 @@
|
|||||||
|
Index: trunk/pjsip/include/pjsip/sip_transaction.h
|
||||||
|
===================================================================
|
||||||
|
--- a/pjsip/include/pjsip/sip_transaction.h (revision 5572)
|
||||||
|
+++ b/pjsip/include/pjsip/sip_transaction.h (revision 5573)
|
||||||
|
@@ -180,4 +180,8 @@
|
||||||
|
* is created by calling #pjsip_tsx_create_key() from an incoming message.
|
||||||
|
*
|
||||||
|
+ * IMPORTANT: To prevent deadlock, application should use
|
||||||
|
+ * #pjsip_tsx_layer_find_tsx2() instead which only adds a reference to
|
||||||
|
+ * the transaction instead of locking it.
|
||||||
|
+ *
|
||||||
|
* @param key The key string to find the transaction.
|
||||||
|
* @param lock If non-zero, transaction will be locked before the
|
||||||
|
@@ -190,4 +194,19 @@
|
||||||
|
PJ_DECL(pjsip_transaction*) pjsip_tsx_layer_find_tsx( const pj_str_t *key,
|
||||||
|
pj_bool_t lock );
|
||||||
|
+
|
||||||
|
+/**
|
||||||
|
+ * Find a transaction with the specified key. The transaction key normally
|
||||||
|
+ * is created by calling #pjsip_tsx_create_key() from an incoming message.
|
||||||
|
+ *
|
||||||
|
+ * @param key The key string to find the transaction.
|
||||||
|
+ * @param add_ref If non-zero, transaction's reference will be added
|
||||||
|
+ * by one before the function returns, to make sure that
|
||||||
|
+ * it's not deleted by other threads.
|
||||||
|
+ *
|
||||||
|
+ * @return The matching transaction instance, or NULL if transaction
|
||||||
|
+ * can not be found.
|
||||||
|
+ */
|
||||||
|
+PJ_DECL(pjsip_transaction*) pjsip_tsx_layer_find_tsx2( const pj_str_t *key,
|
||||||
|
+ pj_bool_t add_ref );
|
||||||
|
|
||||||
|
/**
|
||||||
|
Index: trunk/pjsip/src/pjsip/sip_transaction.c
|
||||||
|
===================================================================
|
||||||
|
--- a/pjsip/src/pjsip/sip_transaction.c (revision 5572)
|
||||||
|
+++ b/pjsip/src/pjsip/sip_transaction.c (revision 5573)
|
||||||
|
@@ -642,6 +642,6 @@
|
||||||
|
* Find a transaction.
|
||||||
|
*/
|
||||||
|
-PJ_DEF(pjsip_transaction*) pjsip_tsx_layer_find_tsx( const pj_str_t *key,
|
||||||
|
- pj_bool_t lock )
|
||||||
|
+static pjsip_transaction* find_tsx( const pj_str_t *key, pj_bool_t lock,
|
||||||
|
+ pj_bool_t add_ref )
|
||||||
|
{
|
||||||
|
pjsip_transaction *tsx;
|
||||||
|
@@ -655,5 +655,5 @@
|
||||||
|
/* Prevent the transaction to get deleted before we have chance to lock it.
|
||||||
|
*/
|
||||||
|
- if (tsx && lock)
|
||||||
|
+ if (tsx)
|
||||||
|
pj_grp_lock_add_ref(tsx->grp_lock);
|
||||||
|
|
||||||
|
@@ -667,10 +667,27 @@
|
||||||
|
PJ_RACE_ME(5);
|
||||||
|
|
||||||
|
- if (tsx && lock) {
|
||||||
|
- pj_grp_lock_acquire(tsx->grp_lock);
|
||||||
|
- pj_grp_lock_dec_ref(tsx->grp_lock);
|
||||||
|
+ if (tsx) {
|
||||||
|
+ if (lock)
|
||||||
|
+ pj_grp_lock_acquire(tsx->grp_lock);
|
||||||
|
+
|
||||||
|
+ if (!add_ref)
|
||||||
|
+ pj_grp_lock_dec_ref(tsx->grp_lock);
|
||||||
|
}
|
||||||
|
|
||||||
|
return tsx;
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+
|
||||||
|
+PJ_DEF(pjsip_transaction*) pjsip_tsx_layer_find_tsx( const pj_str_t *key,
|
||||||
|
+ pj_bool_t lock )
|
||||||
|
+{
|
||||||
|
+ return find_tsx(key, lock, PJ_FALSE);
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+
|
||||||
|
+PJ_DEF(pjsip_transaction*) pjsip_tsx_layer_find_tsx2( const pj_str_t *key,
|
||||||
|
+ pj_bool_t add_ref )
|
||||||
|
+{
|
||||||
|
+ return find_tsx(key, PJ_FALSE, add_ref);
|
||||||
|
}
|
||||||
|
|
||||||
|
Index: trunk/pjsip/src/pjsip/sip_ua_layer.c
|
||||||
|
===================================================================
|
||||||
|
--- a/pjsip/src/pjsip/sip_ua_layer.c (revision 5572)
|
||||||
|
+++ b/pjsip/src/pjsip/sip_ua_layer.c (revision 5573)
|
||||||
|
@@ -552,10 +552,10 @@
|
||||||
|
|
||||||
|
/* Lookup the INVITE transaction */
|
||||||
|
- tsx = pjsip_tsx_layer_find_tsx(&key, PJ_TRUE);
|
||||||
|
+ tsx = pjsip_tsx_layer_find_tsx2(&key, PJ_TRUE);
|
||||||
|
|
||||||
|
/* We should find the dialog attached to the INVITE transaction */
|
||||||
|
if (tsx) {
|
||||||
|
dlg = (pjsip_dialog*) tsx->mod_data[mod_ua.mod.id];
|
||||||
|
- pj_grp_lock_release(tsx->grp_lock);
|
||||||
|
+ pj_grp_lock_dec_ref(tsx->grp_lock);
|
||||||
|
|
||||||
|
/* Dlg may be NULL on some extreme condition
|
||||||
|
Index: trunk/pjsip/src/pjsip-ua/sip_inv.c
|
||||||
|
===================================================================
|
||||||
|
--- a/pjsip/src/pjsip-ua/sip_inv.c (revision 5572)
|
||||||
|
+++ b/pjsip/src/pjsip-ua/sip_inv.c (revision 5573)
|
||||||
|
@@ -3276,5 +3276,5 @@
|
||||||
|
pjsip_tsx_create_key(rdata->tp_info.pool, &key, PJSIP_ROLE_UAS,
|
||||||
|
pjsip_get_invite_method(), rdata);
|
||||||
|
- invite_tsx = pjsip_tsx_layer_find_tsx(&key, PJ_TRUE);
|
||||||
|
+ invite_tsx = pjsip_tsx_layer_find_tsx2(&key, PJ_TRUE);
|
||||||
|
|
||||||
|
if (invite_tsx == NULL) {
|
||||||
|
@@ -3325,5 +3325,5 @@
|
||||||
|
|
||||||
|
if (invite_tsx)
|
||||||
|
- pj_grp_lock_release(invite_tsx->grp_lock);
|
||||||
|
+ pj_grp_lock_dec_ref(invite_tsx->grp_lock);
|
||||||
|
}
|
||||||
|
|
||||||
@ -0,0 +1,72 @@
|
|||||||
|
Index: pjproject/trunk/pjsip/src/pjsip-ua/sip_timer.c
|
||||||
|
===================================================================
|
||||||
|
--- a/pjsip/src/pjsip-ua/sip_timer.c (revision 5557)
|
||||||
|
+++ b/pjsip/src/pjsip-ua/sip_timer.c (revision 5576)
|
||||||
|
@@ -333,6 +333,8 @@
|
||||||
|
pjsip_tx_data *tdata = NULL;
|
||||||
|
pj_status_t status;
|
||||||
|
pj_bool_t as_refresher;
|
||||||
|
+ int entry_id;
|
||||||
|
+ char obj_name[PJ_MAX_OBJ_NAME];
|
||||||
|
|
||||||
|
pj_assert(inv);
|
||||||
|
|
||||||
|
@@ -344,7 +346,10 @@
|
||||||
|
/* Check our role */
|
||||||
|
as_refresher =
|
||||||
|
(inv->timer->refresher == TR_UAC && inv->timer->role == PJSIP_ROLE_UAC) ||
|
||||||
|
- (inv->timer->refresher == TR_UAS && inv->timer->role == PJSIP_ROLE_UAS);
|
||||||
|
+ (inv->timer->refresher == TR_UAS && inv->timer->role == PJSIP_ROLE_UAS);
|
||||||
|
+
|
||||||
|
+ entry_id = entry->id;
|
||||||
|
+ pj_ansi_strncpy(obj_name, inv->pool->obj_name, PJ_MAX_OBJ_NAME);
|
||||||
|
|
||||||
|
/* Do action based on role(refresher or refreshee).
|
||||||
|
* As refresher:
|
||||||
|
@@ -353,7 +358,7 @@
|
||||||
|
* As refreshee:
|
||||||
|
* - end session if there is no refresh request received.
|
||||||
|
*/
|
||||||
|
- if (as_refresher && (entry->id != REFRESHER_EXPIRE_TIMER_ID)) {
|
||||||
|
+ if (as_refresher && (entry_id != REFRESHER_EXPIRE_TIMER_ID)) {
|
||||||
|
pj_time_val now;
|
||||||
|
|
||||||
|
/* As refresher, reshedule the refresh request on the following:
|
||||||
|
@@ -414,7 +419,7 @@
|
||||||
|
}
|
||||||
|
|
||||||
|
pj_gettimeofday(&now);
|
||||||
|
- PJ_LOG(4, (inv->pool->obj_name,
|
||||||
|
+ PJ_LOG(4, (obj_name,
|
||||||
|
"Refreshing session after %ds (expiration period=%ds)",
|
||||||
|
(now.sec-inv->timer->last_refresh.sec),
|
||||||
|
inv->timer->setting.sess_expires));
|
||||||
|
@@ -432,7 +437,7 @@
|
||||||
|
NULL, &tdata);
|
||||||
|
|
||||||
|
pj_gettimeofday(&now);
|
||||||
|
- PJ_LOG(3, (inv->pool->obj_name,
|
||||||
|
+ PJ_LOG(3, (obj_name,
|
||||||
|
"No session %s received after %ds "
|
||||||
|
"(expiration period=%ds), stopping session now!",
|
||||||
|
(as_refresher?"refresh response":"refresh"),
|
||||||
|
@@ -451,11 +456,16 @@
|
||||||
|
status = pjsip_inv_send_msg(inv, tdata);
|
||||||
|
}
|
||||||
|
|
||||||
|
+ /*
|
||||||
|
+ * At this point, dialog might have already been destroyed,
|
||||||
|
+ * including its pool used by the invite session.
|
||||||
|
+ */
|
||||||
|
+
|
||||||
|
/* Print error message, if any */
|
||||||
|
if (status != PJ_SUCCESS) {
|
||||||
|
- PJ_PERROR(2, (inv->pool->obj_name, status,
|
||||||
|
+ PJ_PERROR(2, (obj_name, status,
|
||||||
|
"Error in %s session timer",
|
||||||
|
- ((as_refresher && entry->id != REFRESHER_EXPIRE_TIMER_ID)?
|
||||||
|
+ ((as_refresher && entry_id != REFRESHER_EXPIRE_TIMER_ID)?
|
||||||
|
"refreshing" : "terminating")));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
Loading…
Reference in new issue