1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
|
From 90a16c523bfdf4d43c10506c972c5fd4250b2856 Mon Sep 17 00:00:00 2001
From: Nanang Izzuddin <nanang@teluu.com>
Date: Fri, 20 Nov 2020 10:52:22 +0700
Subject: [PATCH] Race condition between transport destroy and acquire (#2470)
* Handle race condition between transport_idle_callback() and pjsip_tpmgr_acquire_transport2().
* Add transport destroy state check as additional of transport shutdown state check
---
pjsip/src/pjsip/sip_transaction.c | 2 +-
pjsip/src/pjsip/sip_transport.c | 34 +++++++++++++++++++++++++------
2 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/pjsip/src/pjsip/sip_transaction.c b/pjsip/src/pjsip/sip_transaction.c
index 2b4ece7df..f663c7f4b 100644
--- a/pjsip/src/pjsip/sip_transaction.c
+++ b/pjsip/src/pjsip/sip_transaction.c
@@ -2443,7 +2443,7 @@ static void tsx_update_transport( pjsip_transaction *tsx,
pjsip_transport_add_ref(tp);
pjsip_transport_add_state_listener(tp, &tsx_tp_state_callback, tsx,
&tsx->tp_st_key);
- if (tp->is_shutdown) {
+ if (tp->is_shutdown || tp->is_destroying) {
pjsip_transport_state_info info;
pj_bzero(&info, sizeof(info));
diff --git a/pjsip/src/pjsip/sip_transport.c b/pjsip/src/pjsip/sip_transport.c
index 06fce358c..bef6d24fe 100644
--- a/pjsip/src/pjsip/sip_transport.c
+++ b/pjsip/src/pjsip/sip_transport.c
@@ -1071,6 +1071,19 @@ static void transport_idle_callback(pj_timer_heap_t *timer_heap,
return;
entry->id = PJ_FALSE;
+
+ /* Set is_destroying flag under transport manager mutex to avoid
+ * race condition with pjsip_tpmgr_acquire_transport2().
+ */
+ pj_lock_acquire(tp->tpmgr->lock);
+ if (pj_atomic_get(tp->ref_cnt) == 0) {
+ tp->is_destroying = PJ_TRUE;
+ } else {
+ pj_lock_release(tp->tpmgr->lock);
+ return;
+ }
+ pj_lock_release(tp->tpmgr->lock);
+
pjsip_transport_destroy(tp);
}
@@ -1392,8 +1405,8 @@ PJ_DEF(pj_status_t) pjsip_transport_shutdown2(pjsip_transport *tp,
mgr = tp->tpmgr;
pj_lock_acquire(mgr->lock);
- /* Do nothing if transport is being shutdown already */
- if (tp->is_shutdown) {
+ /* Do nothing if transport is being shutdown/destroyed already */
+ if (tp->is_shutdown || tp->is_destroying) {
pj_lock_release(mgr->lock);
pj_lock_release(tp->lock);
return PJ_SUCCESS;
@@ -2256,6 +2269,13 @@ PJ_DEF(pj_status_t) pjsip_tpmgr_acquire_transport2(pjsip_tpmgr *mgr,
return PJSIP_ETPNOTSUITABLE;
}
+ /* Make sure the transport is not being destroyed */
+ if (seltp->is_destroying) {
+ pj_lock_release(mgr->lock);
+ TRACE_((THIS_FILE,"Transport to be acquired is being destroyed"));
+ return PJ_ENOTFOUND;
+ }
+
/* We could also verify that the destination address is reachable
* from this transport (i.e. both are equal), but if application
* has requested a specific transport to be used, assume that
@@ -2311,8 +2331,10 @@ PJ_DEF(pj_status_t) pjsip_tpmgr_acquire_transport2(pjsip_tpmgr *mgr,
if (tp_entry) {
transport *tp_iter = tp_entry;
do {
- /* Don't use transport being shutdown */
- if (!tp_iter->tp->is_shutdown) {
+ /* Don't use transport being shutdown/destroyed */
+ if (!tp_iter->tp->is_shutdown &&
+ !tp_iter->tp->is_destroying)
+ {
if (sel && sel->type == PJSIP_TPSELECTOR_LISTENER &&
sel->u.listener)
{
@@ -2382,7 +2404,7 @@ PJ_DEF(pj_status_t) pjsip_tpmgr_acquire_transport2(pjsip_tpmgr *mgr,
TRACE_((THIS_FILE, "Transport found but from different listener"));
}
- if (tp_ref!=NULL && !tp_ref->is_shutdown) {
+ if (tp_ref!=NULL && !tp_ref->is_shutdown && !tp_ref->is_destroying) {
/*
* Transport found!
*/
@@ -2624,7 +2646,7 @@ PJ_DEF(pj_status_t) pjsip_transport_add_state_listener (
PJ_ASSERT_RETURN(tp && cb && key, PJ_EINVAL);
- if (tp->is_shutdown) {
+ if (tp->is_shutdown || tp->is_destroying) {
*key = NULL;
return PJ_EINVALIDOP;
}
--
2.26.2
|
GitWeb