[Asterisk-code-review] res pjsip: Fix issues that prevented shutdown of modules. (asterisk[15])
Corey Farrell
asteriskteam at digium.com
Wed Oct 4 11:05:18 CDT 2017
Corey Farrell has uploaded this change for review. ( https://gerrit.asterisk.org/6649
Change subject: res_pjsip: Fix issues that prevented shutdown of modules.
......................................................................
res_pjsip: Fix issues that prevented shutdown of modules.
res_pjsip and res_pjsip_session had circular references, preventing both
modules from shutting down.
* Move session supplement registration to res_pjsip.
* Use create internal functions for use by pjsip_message_filter.c.
ASTERISK-27306
Change-Id: Ifbd5c19ec848010111afeab2436f9699da06ba6b
---
M include/asterisk/res_pjsip_session.h
M res/res_pjsip.c
M res/res_pjsip/include/res_pjsip_private.h
M res/res_pjsip/pjsip_message_filter.c
A res/res_pjsip/pjsip_session.c
M res/res_pjsip_session.c
6 files changed, 183 insertions(+), 88 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/49/6649/1
diff --git a/include/asterisk/res_pjsip_session.h b/include/asterisk/res_pjsip_session.h
index fcb14b7..70f9468 100644
--- a/include/asterisk/res_pjsip_session.h
+++ b/include/asterisk/res_pjsip_session.h
@@ -588,6 +588,13 @@
void ast_sip_session_unregister_supplement(struct ast_sip_session_supplement *supplement);
/*!
+ * \brief Add supplements to a SIP session
+ *
+ * \param session The session to initialize
+ */
+int ast_sip_session_add_supplements(struct ast_sip_session *session);
+
+/*!
* \brief Alternative for ast_datastore_alloc()
*
* There are two major differences between this and ast_datastore_alloc()
diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index 6c1f776..ac275bd 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -3516,7 +3516,7 @@
AST_RWLIST_HEAD_STATIC(supplements, ast_sip_supplement);
-int ast_sip_register_supplement(struct ast_sip_supplement *supplement)
+void internal_sip_register_supplement(struct ast_sip_supplement *supplement)
{
struct ast_sip_supplement *iter;
int inserted = 0;
@@ -3534,22 +3534,39 @@
if (!inserted) {
AST_RWLIST_INSERT_TAIL(&supplements, supplement, next);
}
+}
+
+int ast_sip_register_supplement(struct ast_sip_supplement *supplement)
+{
+ internal_sip_register_supplement(supplement);
ast_module_ref(ast_module_info->self);
+
return 0;
}
-void ast_sip_unregister_supplement(struct ast_sip_supplement *supplement)
+int internal_sip_unregister_supplement(struct ast_sip_supplement *supplement)
{
struct ast_sip_supplement *iter;
SCOPED_LOCK(lock, &supplements, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);
+ int res = -1;
+
AST_RWLIST_TRAVERSE_SAFE_BEGIN(&supplements, iter, next) {
if (supplement == iter) {
AST_RWLIST_REMOVE_CURRENT(next);
- ast_module_unref(ast_module_info->self);
+ res = 0;
break;
}
}
AST_RWLIST_TRAVERSE_SAFE_END;
+
+ return res;
+}
+
+void ast_sip_unregister_supplement(struct ast_sip_supplement *supplement)
+{
+ if (!internal_sip_unregister_supplement(supplement)) {
+ ast_module_unref(ast_module_info->self);
+ }
}
static int send_in_dialog_request(pjsip_tx_data *tdata, struct pjsip_dialog *dlg)
diff --git a/res/res_pjsip/include/res_pjsip_private.h b/res/res_pjsip/include/res_pjsip_private.h
index 9fd7aed..5ce3c6f 100644
--- a/res/res_pjsip/include/res_pjsip_private.h
+++ b/res/res_pjsip/include/res_pjsip_private.h
@@ -328,6 +328,18 @@
/*!
* \internal
+ * \brief Used by res_pjsip.so to register a supplement without adding a self reference
+ */
+void internal_sip_register_supplement(struct ast_sip_supplement *supplement);
+
+/*!
+ * \internal
+ * \brief Used by res_pjsip.so to unregister a supplement without removing a self reference
+ */
+int internal_sip_unregister_supplement(struct ast_sip_supplement *supplement);
+
+/*!
+ * \internal
* \brief Used by res_pjsip.so to register an endpoint formatter without adding a self reference
*/
void internal_sip_register_endpoint_formatter(struct ast_sip_endpoint_formatter *obj);
@@ -338,6 +350,20 @@
*/
int internal_sip_unregister_endpoint_formatter(struct ast_sip_endpoint_formatter *obj);
+struct ast_sip_session_supplement;
+
+/*!
+ * \internal
+ * \brief Used by res_pjsip.so to register a session supplement without adding a self reference
+ */
+void internal_sip_session_register_supplement(struct ast_sip_session_supplement *supplement);
+
+/*!
+ * \internal
+ * \brief Used by res_pjsip.so to unregister a session supplement without removing a self reference
+ */
+int internal_sip_session_unregister_supplement(struct ast_sip_session_supplement *supplement);
+
/*!
* \internal
* \brief Finds or creates contact_status for a contact
diff --git a/res/res_pjsip/pjsip_message_filter.c b/res/res_pjsip/pjsip_message_filter.c
index d2f9b95..978aeb0 100644
--- a/res/res_pjsip/pjsip_message_filter.c
+++ b/res/res_pjsip/pjsip_message_filter.c
@@ -505,32 +505,24 @@
void ast_res_pjsip_cleanup_message_filter(void)
{
- ast_sip_unregister_service(&filter_module_tsx);
- ast_sip_unregister_service(&filter_module_transport);
- ast_sip_unregister_supplement(&filter_supplement);
- ast_sip_session_unregister_supplement(&filter_session_supplement);
+ internal_sip_unregister_service(&filter_module_tsx);
+ internal_sip_unregister_service(&filter_module_transport);
+ internal_sip_unregister_supplement(&filter_supplement);
+ internal_sip_session_unregister_supplement(&filter_session_supplement);
}
int ast_res_pjsip_init_message_filter(void)
{
- if (ast_sip_session_register_supplement(&filter_session_supplement)) {
- ast_log(LOG_ERROR, "Could not register message filter session supplement for outgoing requests\n");
- return -1;
- }
+ internal_sip_session_register_supplement(&filter_session_supplement);
+ internal_sip_register_supplement(&filter_supplement);
- if (ast_sip_register_supplement(&filter_supplement)) {
- ast_log(LOG_ERROR, "Could not register message filter supplement for outgoing requests\n");
- ast_res_pjsip_cleanup_message_filter();
- return -1;
- }
-
- if (ast_sip_register_service(&filter_module_transport)) {
+ if (internal_sip_register_service(&filter_module_transport)) {
ast_log(LOG_ERROR, "Could not register message filter module for incoming and outgoing requests\n");
ast_res_pjsip_cleanup_message_filter();
return -1;
}
- if (ast_sip_register_service(&filter_module_tsx)) {
+ if (internal_sip_register_service(&filter_module_tsx)) {
ast_log(LOG_ERROR, "Could not register message filter module for incoming and outgoing requests\n");
ast_res_pjsip_cleanup_message_filter();
return -1;
diff --git a/res/res_pjsip/pjsip_session.c b/res/res_pjsip/pjsip_session.c
new file mode 100644
index 0000000..cea7243
--- /dev/null
+++ b/res/res_pjsip/pjsip_session.c
@@ -0,0 +1,121 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2017, CFWare, LLC
+ *
+ * Corey Farrell <git at cfware.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+#include "asterisk.h"
+
+#include <pjsip.h>
+#include <pjsip_ua.h>
+#include <pjlib.h>
+
+#include "asterisk/res_pjsip.h"
+#include "asterisk/res_pjsip_session.h"
+#include "include/res_pjsip_private.h"
+#include "asterisk/linkedlists.h"
+#include "asterisk/lock.h"
+#include "asterisk/module.h"
+
+
+AST_RWLIST_HEAD_STATIC(session_supplements, ast_sip_session_supplement);
+
+void internal_sip_session_register_supplement(struct ast_sip_session_supplement *supplement)
+{
+ struct ast_sip_session_supplement *iter;
+ int inserted = 0;
+ SCOPED_LOCK(lock, &session_supplements, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);
+
+ if (!supplement->response_priority) {
+ supplement->response_priority = AST_SIP_SESSION_BEFORE_MEDIA;
+ }
+
+ AST_RWLIST_TRAVERSE_SAFE_BEGIN(&session_supplements, iter, next) {
+ if (iter->priority > supplement->priority) {
+ AST_RWLIST_INSERT_BEFORE_CURRENT(supplement, next);
+ inserted = 1;
+ break;
+ }
+ }
+ AST_RWLIST_TRAVERSE_SAFE_END;
+
+ if (!inserted) {
+ AST_RWLIST_INSERT_TAIL(&session_supplements, supplement, next);
+ }
+}
+
+int ast_sip_session_register_supplement(struct ast_sip_session_supplement *supplement)
+{
+ internal_sip_session_register_supplement(supplement);
+ ast_module_ref(AST_MODULE_SELF);
+
+ return 0;
+}
+
+int internal_sip_session_unregister_supplement(struct ast_sip_session_supplement *supplement)
+{
+ struct ast_sip_session_supplement *iter;
+ int res = -1;
+ SCOPED_LOCK(lock, &session_supplements, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);
+
+ AST_RWLIST_TRAVERSE_SAFE_BEGIN(&session_supplements, iter, next) {
+ if (supplement == iter) {
+ AST_RWLIST_REMOVE_CURRENT(next);
+ res = 0;
+ break;
+ }
+ }
+ AST_RWLIST_TRAVERSE_SAFE_END;
+
+ return res;
+}
+
+void ast_sip_session_unregister_supplement(struct ast_sip_session_supplement *supplement)
+{
+ if (!internal_sip_session_unregister_supplement(supplement)) {
+ ast_module_unref(AST_MODULE_SELF);
+ }
+}
+
+static struct ast_sip_session_supplement *supplement_dup(const struct ast_sip_session_supplement *src)
+{
+ struct ast_sip_session_supplement *dst = ast_calloc(1, sizeof(*dst));
+
+ if (!dst) {
+ return NULL;
+ }
+ /* Will need to revisit if shallow copy becomes an issue */
+ *dst = *src;
+
+ return dst;
+}
+
+int ast_sip_session_add_supplements(struct ast_sip_session *session)
+{
+ struct ast_sip_session_supplement *iter;
+ SCOPED_LOCK(lock, &session_supplements, AST_RWLIST_RDLOCK, AST_RWLIST_UNLOCK);
+
+ AST_RWLIST_TRAVERSE(&session_supplements, iter, next) {
+ struct ast_sip_session_supplement *copy = supplement_dup(iter);
+
+ if (!copy) {
+ return -1;
+ }
+ AST_LIST_INSERT_TAIL(&session->supplements, copy, next);
+ }
+
+ return 0;
+}
+
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 70c88a5..fab2757 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -907,59 +907,6 @@
return 0;
}
-AST_RWLIST_HEAD_STATIC(session_supplements, ast_sip_session_supplement);
-
-int ast_sip_session_register_supplement(struct ast_sip_session_supplement *supplement)
-{
- struct ast_sip_session_supplement *iter;
- int inserted = 0;
- SCOPED_LOCK(lock, &session_supplements, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);
-
- if (!supplement->response_priority) {
- supplement->response_priority = AST_SIP_SESSION_BEFORE_MEDIA;
- }
-
- AST_RWLIST_TRAVERSE_SAFE_BEGIN(&session_supplements, iter, next) {
- if (iter->priority > supplement->priority) {
- AST_RWLIST_INSERT_BEFORE_CURRENT(supplement, next);
- inserted = 1;
- break;
- }
- }
- AST_RWLIST_TRAVERSE_SAFE_END;
-
- if (!inserted) {
- AST_RWLIST_INSERT_TAIL(&session_supplements, supplement, next);
- }
- ast_module_ref(ast_module_info->self);
- return 0;
-}
-
-void ast_sip_session_unregister_supplement(struct ast_sip_session_supplement *supplement)
-{
- struct ast_sip_session_supplement *iter;
- SCOPED_LOCK(lock, &session_supplements, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);
- AST_RWLIST_TRAVERSE_SAFE_BEGIN(&session_supplements, iter, next) {
- if (supplement == iter) {
- AST_RWLIST_REMOVE_CURRENT(next);
- ast_module_unref(ast_module_info->self);
- break;
- }
- }
- AST_RWLIST_TRAVERSE_SAFE_END;
-}
-
-static struct ast_sip_session_supplement *supplement_dup(const struct ast_sip_session_supplement *src)
-{
- struct ast_sip_session_supplement *dst = ast_calloc(1, sizeof(*dst));
- if (!dst) {
- return NULL;
- }
- /* Will need to revisit if shallow copy becomes an issue */
- *dst = *src;
- return dst;
-}
-
#define DATASTORE_BUCKETS 53
#define MEDIA_BUCKETS 7
@@ -2045,21 +1992,6 @@
ast_test_suite_event_notify("SESSION_DESTROYED", "Endpoint: %s", endpoint_name);
}
-static int add_supplements(struct ast_sip_session *session)
-{
- struct ast_sip_session_supplement *iter;
- SCOPED_LOCK(lock, &session_supplements, AST_RWLIST_RDLOCK, AST_RWLIST_UNLOCK);
-
- AST_RWLIST_TRAVERSE(&session_supplements, iter, next) {
- struct ast_sip_session_supplement *copy = supplement_dup(iter);
- if (!copy) {
- return -1;
- }
- AST_LIST_INSERT_TAIL(&session->supplements, copy, next);
- }
- return 0;
-}
-
/*! \brief Destructor for SIP channel */
static void sip_channel_destroy(void *obj)
{
@@ -2166,7 +2098,7 @@
session->dtmf = endpoint->dtmf;
- if (add_supplements(session)) {
+ if (ast_sip_session_add_supplements(session)) {
/* Release the ref held by session->inv_session */
ao2_ref(session, -1);
return NULL;
--
To view, visit https://gerrit.asterisk.org/6649
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifbd5c19ec848010111afeab2436f9699da06ba6b
Gerrit-Change-Number: 6649
Gerrit-PatchSet: 1
Gerrit-Owner: Corey Farrell <git at cfware.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171004/f2a75e97/attachment-0001.html>
More information about the asterisk-code-review
mailing list