<p>Friendly Automation <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/10886">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Joshua C. Colp: Looks good to me, but someone else must approve
Kevin Harwell: Looks good to me, approved
Friendly Automation: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">pjsip_transport_management: Shutdown transport immediately on disconnect<br><br>The transport management code that checks for idle connections keeps a<br>reference to PJSIP's transport for IDLE_TIMEOUT milliseconds (32000 by<br>default). Because of this, if the transport is closed before this<br>timeout, the idle checking code will keep the transport from actually<br>being shutdown until the timeout expires.<br><br>Rather than passing the AO2 object to the scheduler task, we just pass<br>its key and look it up when it is time to potentially close the idle<br>connection. The other transport management code handles cleaning up<br>everything else for us.<br><br>Additionally, because we use the address of the transport when<br>generating its name, we concatenate an incrementing ID to the end of the<br>name to guarantee uniqueness.<br><br>Related to ASTERISK~28231<br><br>Change-Id: I02ee9f4073b6abca9169d30c47aa69b5e8ae9afb<br>---<br>M res/res_pjsip/pjsip_transport_management.c<br>M res/res_pjsip_transport_websocket.c<br>2 files changed, 56 insertions(+), 32 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/res/res_pjsip/pjsip_transport_management.c b/res/res_pjsip/pjsip_transport_management.c</span><br><span>index 89ad1fc..ceac889 100644</span><br><span>--- a/res/res_pjsip/pjsip_transport_management.c</span><br><span>+++ b/res/res_pjsip/pjsip_transport_management.c</span><br><span>@@ -139,34 +139,62 @@</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static struct monitored_transport *get_monitored_transport_by_name(const char *obj_name)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ao2_container *transports;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct monitored_transport *monitored = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ transports = ao2_global_obj_ref(monitored_transports);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (transports) {</span><br><span style="color: hsl(120, 100%, 40%);">+ monitored = ao2_find(transports, obj_name, OBJ_SEARCH_KEY);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_cleanup(transports);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Caller is responsible for cleaning up reference */</span><br><span style="color: hsl(120, 100%, 40%);">+ return monitored;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static int idle_sched_cb(const void *data)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct monitored_transport *monitored = (struct monitored_transport *) data;</span><br><span style="color: hsl(120, 100%, 40%);">+ char *obj_name = (char *) data;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct monitored_transport *monitored;</span><br><span> </span><br><span> if (idle_sched_init_pj_thread()) {</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_ref(monitored, -1);</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_free(obj_name);</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!monitored->sip_received) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_NOTICE, "Shutting down transport '%s' since no request was received in %d seconds\n",</span><br><span style="color: hsl(0, 100%, 40%);">- monitored->transport->info, IDLE_TIMEOUT / 1000);</span><br><span style="color: hsl(0, 100%, 40%);">- pjsip_transport_shutdown(monitored->transport);</span><br><span style="color: hsl(120, 100%, 40%);">+ monitored = get_monitored_transport_by_name(obj_name);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (monitored) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!monitored->sip_received) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(LOG_NOTICE, "Shutting down transport '%s' since no request was received in %d seconds\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ monitored->transport->info, IDLE_TIMEOUT / 1000);</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_transport_shutdown(monitored->transport);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(monitored, -1);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ao2_ref(monitored, -1);</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_free(obj_name);</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span> static int idle_sched_cleanup(const void *data)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct monitored_transport *monitored = (struct monitored_transport *) data;</span><br><span style="color: hsl(120, 100%, 40%);">+ char *obj_name = (char *) data;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct monitored_transport *monitored;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!idle_sched_init_pj_thread()) {</span><br><span style="color: hsl(0, 100%, 40%);">- pjsip_transport_shutdown(monitored->transport);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (idle_sched_init_pj_thread()) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_free(obj_name);</span><br><span style="color: hsl(120, 100%, 40%);">+ return 0;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_ref(monitored, -1);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ monitored = get_monitored_transport_by_name(obj_name);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (monitored) {</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_transport_shutdown(monitored->transport);</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(monitored, -1);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_free(obj_name);</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span>@@ -203,13 +231,13 @@</span><br><span> ao2_link(transports, monitored);</span><br><span> </span><br><span> if (transport->dir == PJSIP_TP_DIR_INCOMING) {</span><br><span style="color: hsl(0, 100%, 40%);">- /* Let the scheduler inherit the reference from allocation */</span><br><span style="color: hsl(0, 100%, 40%);">- if (ast_sched_add_variable(sched, IDLE_TIMEOUT, idle_sched_cb, monitored, 1) < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- /* Uh Oh. Could not schedule the idle check. Kill the transport. */</span><br><span style="color: hsl(120, 100%, 40%);">+ char *obj_name = ast_strdup(transport->obj_name);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!obj_name</span><br><span style="color: hsl(120, 100%, 40%);">+ || ast_sched_add_variable(sched, IDLE_TIMEOUT, idle_sched_cb, obj_name, 1) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Shut down the transport if anything fails */</span><br><span> pjsip_transport_shutdown(transport);</span><br><span style="color: hsl(0, 100%, 40%);">- } else {</span><br><span style="color: hsl(0, 100%, 40%);">- /* monitored ref successfully passed to idle_sched_cb() */</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_free(obj_name);</span><br><span> }</span><br><span> }</span><br><span> ao2_ref(monitored, -1);</span><br><span>@@ -324,23 +352,14 @@</span><br><span> */</span><br><span> static pj_bool_t idle_monitor_on_rx_request(pjsip_rx_data *rdata)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct ao2_container *transports;</span><br><span> struct monitored_transport *idle_trans;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- transports = ao2_global_obj_ref(monitored_transports);</span><br><span style="color: hsl(0, 100%, 40%);">- if (!transports) {</span><br><span style="color: hsl(0, 100%, 40%);">- return PJ_FALSE;</span><br><span style="color: hsl(120, 100%, 40%);">+ idle_trans = get_monitored_transport_by_name(rdata->tp_info.transport->obj_name);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (idle_trans) {</span><br><span style="color: hsl(120, 100%, 40%);">+ idle_trans->sip_received = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(idle_trans, -1);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- idle_trans = ao2_find(transports, rdata->tp_info.transport->obj_name, OBJ_SEARCH_KEY);</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_ref(transports, -1);</span><br><span style="color: hsl(0, 100%, 40%);">- if (!idle_trans) {</span><br><span style="color: hsl(0, 100%, 40%);">- return PJ_FALSE;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- idle_trans->sip_received = 1;</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_ref(idle_trans, -1);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> return PJ_FALSE;</span><br><span> }</span><br><span> </span><br><span>diff --git a/res/res_pjsip_transport_websocket.c b/res/res_pjsip_transport_websocket.c</span><br><span>index 6249196..b2e4e1e 100644</span><br><span>--- a/res/res_pjsip_transport_websocket.c</span><br><span>+++ b/res/res_pjsip_transport_websocket.c</span><br><span>@@ -42,6 +42,11 @@</span><br><span> static int transport_type_wss_ipv6;</span><br><span> </span><br><span> /*!</span><br><span style="color: hsl(120, 100%, 40%);">+ * Used to ensure uniqueness among WS transport names</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+static int ws_obj_name_serial;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/*!</span><br><span> * \brief Wrapper for pjsip_transport, for storing the WebSocket session</span><br><span> */</span><br><span> struct ws_transport {</span><br><span>@@ -163,8 +168,8 @@</span><br><span> }</span><br><span> </span><br><span> /* Give websocket transport a unique name for its lifetime */</span><br><span style="color: hsl(0, 100%, 40%);">- snprintf(newtransport->transport.obj_name, PJ_MAX_OBJ_NAME, "ws%p",</span><br><span style="color: hsl(0, 100%, 40%);">- &newtransport->transport);</span><br><span style="color: hsl(120, 100%, 40%);">+ snprintf(newtransport->transport.obj_name, PJ_MAX_OBJ_NAME, "ws%p-%d",</span><br><span style="color: hsl(120, 100%, 40%);">+ &newtransport->transport, ast_atomic_fetchadd_int(&ws_obj_name_serial, 1));</span><br><span> </span><br><span> newtransport->transport.endpt = endpt;</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/10886">change 10886</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/10886"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I02ee9f4073b6abca9169d30c47aa69b5e8ae9afb </div>
<div style="display:none"> Gerrit-Change-Number: 10886 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: Sean Bright <sean.bright@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation (1000185) </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua C. Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Sean Bright <sean.bright@gmail.com> </div>