[Asterisk-code-review] pjsip transport management: Shutdown transport immediately o... (asterisk[13])
Friendly Automation
asteriskteam at digium.com
Tue Jan 22 18:24:38 CST 2019
Friendly Automation has submitted this change and it was merged. ( https://gerrit.asterisk.org/10886 )
Change subject: pjsip_transport_management: Shutdown transport immediately on disconnect
......................................................................
pjsip_transport_management: Shutdown transport immediately on disconnect
The transport management code that checks for idle connections keeps a
reference to PJSIP's transport for IDLE_TIMEOUT milliseconds (32000 by
default). Because of this, if the transport is closed before this
timeout, the idle checking code will keep the transport from actually
being shutdown until the timeout expires.
Rather than passing the AO2 object to the scheduler task, we just pass
its key and look it up when it is time to potentially close the idle
connection. The other transport management code handles cleaning up
everything else for us.
Additionally, because we use the address of the transport when
generating its name, we concatenate an incrementing ID to the end of the
name to guarantee uniqueness.
Related to ASTERISK~28231
Change-Id: I02ee9f4073b6abca9169d30c47aa69b5e8ae9afb
---
M res/res_pjsip/pjsip_transport_management.c
M res/res_pjsip_transport_websocket.c
2 files changed, 56 insertions(+), 32 deletions(-)
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
diff --git a/res/res_pjsip/pjsip_transport_management.c b/res/res_pjsip/pjsip_transport_management.c
index 89ad1fc..ceac889 100644
--- a/res/res_pjsip/pjsip_transport_management.c
+++ b/res/res_pjsip/pjsip_transport_management.c
@@ -139,34 +139,62 @@
return 0;
}
+static struct monitored_transport *get_monitored_transport_by_name(const char *obj_name)
+{
+ struct ao2_container *transports;
+ struct monitored_transport *monitored = NULL;
+
+ transports = ao2_global_obj_ref(monitored_transports);
+ if (transports) {
+ monitored = ao2_find(transports, obj_name, OBJ_SEARCH_KEY);
+ }
+ ao2_cleanup(transports);
+
+ /* Caller is responsible for cleaning up reference */
+ return monitored;
+}
+
static int idle_sched_cb(const void *data)
{
- struct monitored_transport *monitored = (struct monitored_transport *) data;
+ char *obj_name = (char *) data;
+ struct monitored_transport *monitored;
if (idle_sched_init_pj_thread()) {
- ao2_ref(monitored, -1);
+ ast_free(obj_name);
return 0;
}
- if (!monitored->sip_received) {
- ast_log(LOG_NOTICE, "Shutting down transport '%s' since no request was received in %d seconds\n",
- monitored->transport->info, IDLE_TIMEOUT / 1000);
- pjsip_transport_shutdown(monitored->transport);
+ monitored = get_monitored_transport_by_name(obj_name);
+ if (monitored) {
+ if (!monitored->sip_received) {
+ ast_log(LOG_NOTICE, "Shutting down transport '%s' since no request was received in %d seconds\n",
+ monitored->transport->info, IDLE_TIMEOUT / 1000);
+ pjsip_transport_shutdown(monitored->transport);
+ }
+ ao2_ref(monitored, -1);
}
- ao2_ref(monitored, -1);
+ ast_free(obj_name);
return 0;
}
static int idle_sched_cleanup(const void *data)
{
- struct monitored_transport *monitored = (struct monitored_transport *) data;
+ char *obj_name = (char *) data;
+ struct monitored_transport *monitored;
- if (!idle_sched_init_pj_thread()) {
- pjsip_transport_shutdown(monitored->transport);
+ if (idle_sched_init_pj_thread()) {
+ ast_free(obj_name);
+ return 0;
}
- ao2_ref(monitored, -1);
+ monitored = get_monitored_transport_by_name(obj_name);
+ if (monitored) {
+ pjsip_transport_shutdown(monitored->transport);
+ ao2_ref(monitored, -1);
+ }
+
+ ast_free(obj_name);
return 0;
}
@@ -203,13 +231,13 @@
ao2_link(transports, monitored);
if (transport->dir == PJSIP_TP_DIR_INCOMING) {
- /* Let the scheduler inherit the reference from allocation */
- if (ast_sched_add_variable(sched, IDLE_TIMEOUT, idle_sched_cb, monitored, 1) < 0) {
- /* Uh Oh. Could not schedule the idle check. Kill the transport. */
+ char *obj_name = ast_strdup(transport->obj_name);
+
+ if (!obj_name
+ || ast_sched_add_variable(sched, IDLE_TIMEOUT, idle_sched_cb, obj_name, 1) < 0) {
+ /* Shut down the transport if anything fails */
pjsip_transport_shutdown(transport);
- } else {
- /* monitored ref successfully passed to idle_sched_cb() */
- break;
+ ast_free(obj_name);
}
}
ao2_ref(monitored, -1);
@@ -324,23 +352,14 @@
*/
static pj_bool_t idle_monitor_on_rx_request(pjsip_rx_data *rdata)
{
- struct ao2_container *transports;
struct monitored_transport *idle_trans;
- transports = ao2_global_obj_ref(monitored_transports);
- if (!transports) {
- return PJ_FALSE;
+ idle_trans = get_monitored_transport_by_name(rdata->tp_info.transport->obj_name);
+ if (idle_trans) {
+ idle_trans->sip_received = 1;
+ ao2_ref(idle_trans, -1);
}
- idle_trans = ao2_find(transports, rdata->tp_info.transport->obj_name, OBJ_SEARCH_KEY);
- ao2_ref(transports, -1);
- if (!idle_trans) {
- return PJ_FALSE;
- }
-
- idle_trans->sip_received = 1;
- ao2_ref(idle_trans, -1);
-
return PJ_FALSE;
}
diff --git a/res/res_pjsip_transport_websocket.c b/res/res_pjsip_transport_websocket.c
index 6249196..b2e4e1e 100644
--- a/res/res_pjsip_transport_websocket.c
+++ b/res/res_pjsip_transport_websocket.c
@@ -42,6 +42,11 @@
static int transport_type_wss_ipv6;
/*!
+ * Used to ensure uniqueness among WS transport names
+ */
+static int ws_obj_name_serial;
+
+/*!
* \brief Wrapper for pjsip_transport, for storing the WebSocket session
*/
struct ws_transport {
@@ -163,8 +168,8 @@
}
/* Give websocket transport a unique name for its lifetime */
- snprintf(newtransport->transport.obj_name, PJ_MAX_OBJ_NAME, "ws%p",
- &newtransport->transport);
+ snprintf(newtransport->transport.obj_name, PJ_MAX_OBJ_NAME, "ws%p-%d",
+ &newtransport->transport, ast_atomic_fetchadd_int(&ws_obj_name_serial, 1));
newtransport->transport.endpt = endpt;
--
To view, visit https://gerrit.asterisk.org/10886
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: merged
Gerrit-Change-Id: I02ee9f4073b6abca9169d30c47aa69b5e8ae9afb
Gerrit-Change-Number: 10886
Gerrit-PatchSet: 5
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: Friendly Automation (1000185)
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua C. Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190122/e1a3dacd/attachment.html>
More information about the asterisk-code-review
mailing list