<p>Corey Farrell has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/6692">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_pjsip: Fix issues that prevented shutdown of modules.<br><br>res_pjsip and res_pjsip_session had circular references, preventing both<br>modules from shutting down.<br>* Move session supplement registration to res_pjsip.<br>* Use create internal functions for use by pjsip_message_filter.c.<br><br>ASTERISK-27306<br><br>Change-Id: Ifbd5c19ec848010111afeab2436f9699da06ba6b<br>---<br>M include/asterisk/res_pjsip_session.h<br>M res/res_pjsip.c<br>M res/res_pjsip/include/res_pjsip_private.h<br>M res/res_pjsip/pjsip_message_filter.c<br>A res/res_pjsip/pjsip_session.c<br>M res/res_pjsip_session.c<br>6 files changed, 183 insertions(+), 88 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/92/6692/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/include/asterisk/res_pjsip_session.h b/include/asterisk/res_pjsip_session.h<br>index d275c25..7d6b302 100644<br>--- a/include/asterisk/res_pjsip_session.h<br>+++ b/include/asterisk/res_pjsip_session.h<br>@@ -544,6 +544,13 @@<br> void ast_sip_session_unregister_supplement(struct ast_sip_session_supplement *supplement);<br> <br> /*!<br>+ * \brief Add supplements to a SIP session<br>+ *<br>+ * \param session The session to initialize<br>+ */<br>+int ast_sip_session_add_supplements(struct ast_sip_session *session);<br>+<br>+/*!<br> * \brief Alternative for ast_datastore_alloc()<br> *<br> * There are two major differences between this and ast_datastore_alloc()<br>diff --git a/res/res_pjsip.c b/res/res_pjsip.c<br>index 4d5c5cb..844d8a1 100644<br>--- a/res/res_pjsip.c<br>+++ b/res/res_pjsip.c<br>@@ -3476,7 +3476,7 @@<br> <br> AST_RWLIST_HEAD_STATIC(supplements, ast_sip_supplement);<br> <br>-int ast_sip_register_supplement(struct ast_sip_supplement *supplement)<br>+void internal_sip_register_supplement(struct ast_sip_supplement *supplement)<br> {<br> struct ast_sip_supplement *iter;<br> int inserted = 0;<br>@@ -3494,22 +3494,39 @@<br> if (!inserted) {<br> AST_RWLIST_INSERT_TAIL(&supplements, supplement, next);<br> }<br>+}<br>+<br>+int ast_sip_register_supplement(struct ast_sip_supplement *supplement)<br>+{<br>+ internal_sip_register_supplement(supplement);<br> ast_module_ref(ast_module_info->self);<br>+<br> return 0;<br> }<br> <br>-void ast_sip_unregister_supplement(struct ast_sip_supplement *supplement)<br>+int internal_sip_unregister_supplement(struct ast_sip_supplement *supplement)<br> {<br> struct ast_sip_supplement *iter;<br> SCOPED_LOCK(lock, &supplements, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);<br>+ int res = -1;<br>+<br> AST_RWLIST_TRAVERSE_SAFE_BEGIN(&supplements, iter, next) {<br> if (supplement == iter) {<br> AST_RWLIST_REMOVE_CURRENT(next);<br>- ast_module_unref(ast_module_info->self);<br>+ res = 0;<br> break;<br> }<br> }<br> AST_RWLIST_TRAVERSE_SAFE_END;<br>+<br>+ return res;<br>+}<br>+<br>+void ast_sip_unregister_supplement(struct ast_sip_supplement *supplement)<br>+{<br>+ if (!internal_sip_unregister_supplement(supplement)) {<br>+ ast_module_unref(ast_module_info->self);<br>+ }<br> }<br> <br> static int send_in_dialog_request(pjsip_tx_data *tdata, struct pjsip_dialog *dlg)<br>diff --git a/res/res_pjsip/include/res_pjsip_private.h b/res/res_pjsip/include/res_pjsip_private.h<br>index 9fd7aed..5ce3c6f 100644<br>--- a/res/res_pjsip/include/res_pjsip_private.h<br>+++ b/res/res_pjsip/include/res_pjsip_private.h<br>@@ -328,6 +328,18 @@<br> <br> /*!<br> * \internal<br>+ * \brief Used by res_pjsip.so to register a supplement without adding a self reference<br>+ */<br>+void internal_sip_register_supplement(struct ast_sip_supplement *supplement);<br>+<br>+/*!<br>+ * \internal<br>+ * \brief Used by res_pjsip.so to unregister a supplement without removing a self reference<br>+ */<br>+int internal_sip_unregister_supplement(struct ast_sip_supplement *supplement);<br>+<br>+/*!<br>+ * \internal<br> * \brief Used by res_pjsip.so to register an endpoint formatter without adding a self reference<br> */<br> void internal_sip_register_endpoint_formatter(struct ast_sip_endpoint_formatter *obj);<br>@@ -338,6 +350,20 @@<br> */<br> int internal_sip_unregister_endpoint_formatter(struct ast_sip_endpoint_formatter *obj);<br> <br>+struct ast_sip_session_supplement;<br>+<br>+/*!<br>+ * \internal<br>+ * \brief Used by res_pjsip.so to register a session supplement without adding a self reference<br>+ */<br>+void internal_sip_session_register_supplement(struct ast_sip_session_supplement *supplement);<br>+<br>+/*!<br>+ * \internal<br>+ * \brief Used by res_pjsip.so to unregister a session supplement without removing a self reference<br>+ */<br>+int internal_sip_session_unregister_supplement(struct ast_sip_session_supplement *supplement);<br>+<br> /*!<br> * \internal<br> * \brief Finds or creates contact_status for a contact<br>diff --git a/res/res_pjsip/pjsip_message_filter.c b/res/res_pjsip/pjsip_message_filter.c<br>index d2f9b95..978aeb0 100644<br>--- a/res/res_pjsip/pjsip_message_filter.c<br>+++ b/res/res_pjsip/pjsip_message_filter.c<br>@@ -505,32 +505,24 @@<br> <br> void ast_res_pjsip_cleanup_message_filter(void)<br> {<br>- ast_sip_unregister_service(&filter_module_tsx);<br>- ast_sip_unregister_service(&filter_module_transport);<br>- ast_sip_unregister_supplement(&filter_supplement);<br>- ast_sip_session_unregister_supplement(&filter_session_supplement);<br>+ internal_sip_unregister_service(&filter_module_tsx);<br>+ internal_sip_unregister_service(&filter_module_transport);<br>+ internal_sip_unregister_supplement(&filter_supplement);<br>+ internal_sip_session_unregister_supplement(&filter_session_supplement);<br> }<br> <br> int ast_res_pjsip_init_message_filter(void)<br> {<br>- if (ast_sip_session_register_supplement(&filter_session_supplement)) {<br>- ast_log(LOG_ERROR, "Could not register message filter session supplement for outgoing requests\n");<br>- return -1;<br>- }<br>+ internal_sip_session_register_supplement(&filter_session_supplement);<br>+ internal_sip_register_supplement(&filter_supplement);<br> <br>- if (ast_sip_register_supplement(&filter_supplement)) {<br>- ast_log(LOG_ERROR, "Could not register message filter supplement for outgoing requests\n");<br>- ast_res_pjsip_cleanup_message_filter();<br>- return -1;<br>- }<br>-<br>- if (ast_sip_register_service(&filter_module_transport)) {<br>+ if (internal_sip_register_service(&filter_module_transport)) {<br> ast_log(LOG_ERROR, "Could not register message filter module for incoming and outgoing requests\n");<br> ast_res_pjsip_cleanup_message_filter();<br> return -1;<br> }<br> <br>- if (ast_sip_register_service(&filter_module_tsx)) {<br>+ if (internal_sip_register_service(&filter_module_tsx)) {<br> ast_log(LOG_ERROR, "Could not register message filter module for incoming and outgoing requests\n");<br> ast_res_pjsip_cleanup_message_filter();<br> return -1;<br>diff --git a/res/res_pjsip/pjsip_session.c b/res/res_pjsip/pjsip_session.c<br>new file mode 100644<br>index 0000000..cea7243<br>--- /dev/null<br>+++ b/res/res_pjsip/pjsip_session.c<br>@@ -0,0 +1,121 @@<br>+/*<br>+ * Asterisk -- An open source telephony toolkit.<br>+ *<br>+ * Copyright (C) 2017, CFWare, LLC<br>+ *<br>+ * Corey Farrell <git@cfware.com><br>+ *<br>+ * See http://www.asterisk.org for more information about<br>+ * the Asterisk project. Please do not directly contact<br>+ * any of the maintainers of this project for assistance;<br>+ * the project provides a web site, mailing lists and IRC<br>+ * channels for your use.<br>+ *<br>+ * This program is free software, distributed under the terms of<br>+ * the GNU General Public License Version 2. See the LICENSE file<br>+ * at the top of the source tree.<br>+ */<br>+<br>+#include "asterisk.h"<br>+<br>+#include <pjsip.h><br>+#include <pjsip_ua.h><br>+#include <pjlib.h><br>+<br>+#include "asterisk/res_pjsip.h"<br>+#include "asterisk/res_pjsip_session.h"<br>+#include "include/res_pjsip_private.h"<br>+#include "asterisk/linkedlists.h"<br>+#include "asterisk/lock.h"<br>+#include "asterisk/module.h"<br>+<br>+<br>+AST_RWLIST_HEAD_STATIC(session_supplements, ast_sip_session_supplement);<br>+<br>+void internal_sip_session_register_supplement(struct ast_sip_session_supplement *supplement)<br>+{<br>+ struct ast_sip_session_supplement *iter;<br>+ int inserted = 0;<br>+ SCOPED_LOCK(lock, &session_supplements, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);<br>+<br>+ if (!supplement->response_priority) {<br>+ supplement->response_priority = AST_SIP_SESSION_BEFORE_MEDIA;<br>+ }<br>+<br>+ AST_RWLIST_TRAVERSE_SAFE_BEGIN(&session_supplements, iter, next) {<br>+ if (iter->priority > supplement->priority) {<br>+ AST_RWLIST_INSERT_BEFORE_CURRENT(supplement, next);<br>+ inserted = 1;<br>+ break;<br>+ }<br>+ }<br>+ AST_RWLIST_TRAVERSE_SAFE_END;<br>+<br>+ if (!inserted) {<br>+ AST_RWLIST_INSERT_TAIL(&session_supplements, supplement, next);<br>+ }<br>+}<br>+<br>+int ast_sip_session_register_supplement(struct ast_sip_session_supplement *supplement)<br>+{<br>+ internal_sip_session_register_supplement(supplement);<br>+ ast_module_ref(AST_MODULE_SELF);<br>+<br>+ return 0;<br>+}<br>+<br>+int internal_sip_session_unregister_supplement(struct ast_sip_session_supplement *supplement)<br>+{<br>+ struct ast_sip_session_supplement *iter;<br>+ int res = -1;<br>+ SCOPED_LOCK(lock, &session_supplements, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);<br>+<br>+ AST_RWLIST_TRAVERSE_SAFE_BEGIN(&session_supplements, iter, next) {<br>+ if (supplement == iter) {<br>+ AST_RWLIST_REMOVE_CURRENT(next);<br>+ res = 0;<br>+ break;<br>+ }<br>+ }<br>+ AST_RWLIST_TRAVERSE_SAFE_END;<br>+<br>+ return res;<br>+}<br>+<br>+void ast_sip_session_unregister_supplement(struct ast_sip_session_supplement *supplement)<br>+{<br>+ if (!internal_sip_session_unregister_supplement(supplement)) {<br>+ ast_module_unref(AST_MODULE_SELF);<br>+ }<br>+}<br>+<br>+static struct ast_sip_session_supplement *supplement_dup(const struct ast_sip_session_supplement *src)<br>+{<br>+ struct ast_sip_session_supplement *dst = ast_calloc(1, sizeof(*dst));<br>+<br>+ if (!dst) {<br>+ return NULL;<br>+ }<br>+ /* Will need to revisit if shallow copy becomes an issue */<br>+ *dst = *src;<br>+<br>+ return dst;<br>+}<br>+<br>+int ast_sip_session_add_supplements(struct ast_sip_session *session)<br>+{<br>+ struct ast_sip_session_supplement *iter;<br>+ SCOPED_LOCK(lock, &session_supplements, AST_RWLIST_RDLOCK, AST_RWLIST_UNLOCK);<br>+<br>+ AST_RWLIST_TRAVERSE(&session_supplements, iter, next) {<br>+ struct ast_sip_session_supplement *copy = supplement_dup(iter);<br>+<br>+ if (!copy) {<br>+ return -1;<br>+ }<br>+ AST_LIST_INSERT_TAIL(&session->supplements, copy, next);<br>+ }<br>+<br>+ return 0;<br>+}<br>+<br>diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c<br>index 496c476..f84d60e 100644<br>--- a/res/res_pjsip_session.c<br>+++ b/res/res_pjsip_session.c<br>@@ -396,59 +396,6 @@<br> return -1;<br> }<br> <br>-AST_RWLIST_HEAD_STATIC(session_supplements, ast_sip_session_supplement);<br>-<br>-int ast_sip_session_register_supplement(struct ast_sip_session_supplement *supplement)<br>-{<br>- struct ast_sip_session_supplement *iter;<br>- int inserted = 0;<br>- SCOPED_LOCK(lock, &session_supplements, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);<br>-<br>- if (!supplement->response_priority) {<br>- supplement->response_priority = AST_SIP_SESSION_BEFORE_MEDIA;<br>- }<br>-<br>- AST_RWLIST_TRAVERSE_SAFE_BEGIN(&session_supplements, iter, next) {<br>- if (iter->priority > supplement->priority) {<br>- AST_RWLIST_INSERT_BEFORE_CURRENT(supplement, next);<br>- inserted = 1;<br>- break;<br>- }<br>- }<br>- AST_RWLIST_TRAVERSE_SAFE_END;<br>-<br>- if (!inserted) {<br>- AST_RWLIST_INSERT_TAIL(&session_supplements, supplement, next);<br>- }<br>- ast_module_ref(ast_module_info->self);<br>- return 0;<br>-}<br>-<br>-void ast_sip_session_unregister_supplement(struct ast_sip_session_supplement *supplement)<br>-{<br>- struct ast_sip_session_supplement *iter;<br>- SCOPED_LOCK(lock, &session_supplements, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);<br>- AST_RWLIST_TRAVERSE_SAFE_BEGIN(&session_supplements, iter, next) {<br>- if (supplement == iter) {<br>- AST_RWLIST_REMOVE_CURRENT(next);<br>- ast_module_unref(ast_module_info->self);<br>- break;<br>- }<br>- }<br>- AST_RWLIST_TRAVERSE_SAFE_END;<br>-}<br>-<br>-static struct ast_sip_session_supplement *supplement_dup(const struct ast_sip_session_supplement *src)<br>-{<br>- struct ast_sip_session_supplement *dst = ast_calloc(1, sizeof(*dst));<br>- if (!dst) {<br>- return NULL;<br>- }<br>- /* Will need to revisit if shallow copy becomes an issue */<br>- *dst = *src;<br>- return dst;<br>-}<br>-<br> #define DATASTORE_BUCKETS 53<br> #define MEDIA_BUCKETS 7<br> <br>@@ -1380,21 +1327,6 @@<br> ast_test_suite_event_notify("SESSION_DESTROYED", "Endpoint: %s", endpoint_name);<br> }<br> <br>-static int add_supplements(struct ast_sip_session *session)<br>-{<br>- struct ast_sip_session_supplement *iter;<br>- SCOPED_LOCK(lock, &session_supplements, AST_RWLIST_RDLOCK, AST_RWLIST_UNLOCK);<br>-<br>- AST_RWLIST_TRAVERSE(&session_supplements, iter, next) {<br>- struct ast_sip_session_supplement *copy = supplement_dup(iter);<br>- if (!copy) {<br>- return -1;<br>- }<br>- AST_LIST_INSERT_TAIL(&session->supplements, copy, next);<br>- }<br>- return 0;<br>-}<br>-<br> static int add_session_media(void *obj, void *arg, int flags)<br> {<br> struct sdp_handler_list *handler_list = obj;<br>@@ -1523,7 +1455,7 @@<br> <br> session->dtmf = endpoint->dtmf;<br> <br>- if (add_supplements(session)) {<br>+ if (ast_sip_session_add_supplements(session)) {<br> /* Release the ref held by session->inv_session */<br> ao2_ref(session, -1);<br> return NULL;<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6692">change 6692</a>. To unsubscribe, 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/6692"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 14 </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Ifbd5c19ec848010111afeab2436f9699da06ba6b </div>
<div style="display:none"> Gerrit-Change-Number: 6692 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Corey Farrell <git@cfware.com> </div>