<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>