[Asterisk-code-review] res pjsip: Patch for res pjsip * module load/reload crash (asterisk[master])
sungtae kim
asteriskteam at digium.com
Sun Nov 11 10:29:10 CST 2018
sungtae kim has uploaded this change for review. ( https://gerrit.asterisk.org/10617
Change subject: res_pjsip: Patch for res_pjsip_* module load/reload crash
......................................................................
res_pjsip: Patch for res_pjsip_* module load/reload crash
The session_supplements for the pjsip makes crashes when the module
load/unload.
ASTERISK-28157
Change-Id: I5b82be3a75d702cf1933d8d1417f44aa10ad1029
---
M include/asterisk/res_pjsip_session.h
M res/res_pjsip/pjsip_session.c
M res/res_pjsip_session.c
3 files changed, 306 insertions(+), 19 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/17/10617/1
diff --git a/include/asterisk/res_pjsip_session.h b/include/asterisk/res_pjsip_session.h
index ccee8c9..5b948b4 100644
--- a/include/asterisk/res_pjsip_session.h
+++ b/include/asterisk/res_pjsip_session.h
@@ -215,6 +215,9 @@
enum ast_sip_dtmf_mode dtmf;
/*! Initial incoming INVITE Request-URI. NULL otherwise. */
pjsip_uri *request_uri;
+
+ /*! Next item in the list */
+ AST_LIST_ENTRY(ast_sip_session) next;
};
typedef int (*ast_sip_session_request_creation_cb)(struct ast_sip_session *session, pjsip_tx_data *tdata);
@@ -472,6 +475,13 @@
struct ast_sip_contact *contact, pjsip_inv_session *inv, pjsip_rx_data *rdata);
/*!
+ * \brief Unregister a an supplement to SIP session processing from exisiting SIP sessions.
+ *
+ * \param supplement The supplement to unregister
+ */
+void ast_sip_remove_registered_supplement(struct ast_sip_session_supplement *supplement);
+
+/*!
* \brief Request and wait for the session serializer to be suspended.
* \since 12.7.0
*
@@ -573,6 +583,21 @@
void ast_sip_session_unregister_sdp_handler(struct ast_sip_session_sdp_handler *handler, const char *stream_type);
/*!
+ * \brief Insert a session to the list.
+ *
+ * \param session
+ */
+void ast_sip_session_insert(struct ast_sip_session *session);
+
+/*!
+ * \brief Remove session from the list.
+ *
+ * \param session
+ */
+void ast_sip_session_remove(struct ast_sip_session *session);
+
+
+/*!
* \brief Register a supplement to SIP session processing
*
* This allows for someone to insert themselves in the processing of SIP
diff --git a/res/res_pjsip/pjsip_session.c b/res/res_pjsip/pjsip_session.c
index f3f3a4d..95c8fb6 100644
--- a/res/res_pjsip/pjsip_session.c
+++ b/res/res_pjsip/pjsip_session.c
@@ -29,9 +29,14 @@
#include "asterisk/lock.h"
#include "asterisk/module.h"
+AST_LIST_HEAD_STATIC(pjsip_sessions, ast_sip_session);
AST_RWLIST_HEAD_STATIC(session_supplements, ast_sip_session_supplement);
+static void remove_registered_supplement(struct ast_sip_session *session, struct ast_sip_session_supplement *supplement);
+static int is_same_supplement(const struct ast_sip_session_supplement *supp1, const struct ast_sip_session_supplement *supp2);
+static int is_same_method(const char* m1, const char* m2);
+
void ast_sip_session_register_supplement(struct ast_sip_session_supplement *supplement)
{
struct ast_sip_session_supplement *iter;
@@ -59,7 +64,8 @@
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_WRLOCK(&session_supplements);
AST_RWLIST_TRAVERSE_SAFE_BEGIN(&session_supplements, iter, next) {
if (supplement == iter) {
@@ -68,6 +74,10 @@
}
}
AST_RWLIST_TRAVERSE_SAFE_END;
+ AST_RWLIST_UNLOCK(&session_supplements);
+
+ ast_sip_remove_registered_supplement(supplement);
+
}
static struct ast_sip_session_supplement *supplement_dup(const struct ast_sip_session_supplement *src)
@@ -77,6 +87,7 @@
if (!dst) {
return NULL;
}
+
/* Will need to revisit if shallow copy becomes an issue */
*dst = *src;
@@ -94,8 +105,183 @@
if (!copy) {
return -1;
}
+
+ AST_LIST_LOCK(&session->supplements);
AST_LIST_INSERT_TAIL(&session->supplements, copy, next);
+ AST_LIST_UNLOCK(&session->supplements);
}
return 0;
}
+
+void ast_sip_remove_registered_supplement(struct ast_sip_session_supplement *supplement)
+{
+ struct ast_sip_session *iter;
+
+ if (!supplement) {
+ return;
+ }
+
+ AST_LIST_LOCK(&pjsip_sessions);
+
+ AST_LIST_TRAVERSE_SAFE_BEGIN(&pjsip_sessions, iter, next) {
+ remove_registered_supplement(iter, supplement);
+ }
+ AST_LIST_TRAVERSE_SAFE_END;
+
+ AST_LIST_UNLOCK(&pjsip_sessions);
+}
+
+/*!
+ * Returns 1 if the given string is the same.
+ * \param m1
+ * \param m2
+ * \return
+ */
+static int is_same_method(const char* m1, const char* m2)
+{
+ int ret;
+
+ if (m1 == m2) {
+ return 1;
+ }
+
+ if ((!m1) || (!m2)) {
+ return 0;
+ }
+
+ ret = strcmp(m1, m2);
+ if (ret != 0) {
+ return 0;
+ }
+
+ return 1;
+}
+
+/*!
+ *
+ * \param supp1
+ * \param supp2
+ * \return return 1 if supplements are the same. Otherwise 0.
+ */
+static int is_same_supplement(const struct ast_sip_session_supplement *supp1, const struct ast_sip_session_supplement *supp2)
+{
+ if ((!supp1) || (!supp2)) {
+ return 1;
+ }
+
+ /* check method */
+ if (is_same_method(supp1->method, supp2->method) == 0) {
+ return 0;
+ }
+
+ /* check request/response handler */
+ if ((supp1->incoming_request != supp2->incoming_request)
+ || (supp1->incoming_response != supp2->incoming_response)
+ || (supp1->outgoing_request != supp2->outgoing_request)
+ || (supp1->outgoing_response != supp2->outgoing_response)
+ ) {
+ return 0;
+ }
+
+ /* check session handler */
+ if ((supp1->session_begin != supp2->session_begin)
+ || (supp1->session_destroy != supp2->session_destroy)
+ || (supp1->session_end != supp2->session_end)
+ ) {
+ return 0;
+ }
+
+ /* check priority */
+ if ((supp1->priority != supp2->priority)
+ || (supp1->response_priority != supp2->response_priority)
+ ) {
+ return 0;
+ }
+
+ return 1;
+
+}
+
+static void remove_registered_supplement(struct ast_sip_session *session, struct ast_sip_session_supplement *supplement)
+{
+ struct ast_sip_session_supplement *iter;
+
+ if (!session) {
+ return;
+ }
+ ast_log(LOG_DEBUG, "Removing registered supplement. method[%s]\n", supplement->method);
+
+ AST_LIST_LOCK(&session->supplements);
+
+ AST_LIST_TRAVERSE_SAFE_BEGIN(&session->supplements, iter, next) {
+ if (is_same_supplement(iter, supplement)) {
+ ast_log(LOG_DEBUG, "Found same supplement. Remove it.\n");
+ AST_LIST_REMOVE_CURRENT(next);
+ break;
+ }
+
+ }
+ AST_LIST_TRAVERSE_SAFE_END;
+
+ AST_LIST_UNLOCK(&session->supplements);
+
+ return;
+}
+
+void ast_sip_session_insert(struct ast_sip_session *session)
+{
+ struct ast_sip_session *iter;
+ int inserted;
+ int count;
+
+ if (!session) {
+ return;
+ }
+
+ inserted = 0;
+ count = 0;
+
+ AST_LIST_LOCK(&pjsip_sessions);
+
+ AST_LIST_TRAVERSE_SAFE_BEGIN(&pjsip_sessions, iter, next) {
+ if (iter == session) {
+ inserted = 1;
+ break;
+ }
+ count++;
+ }
+ AST_LIST_TRAVERSE_SAFE_END;
+
+ if (!inserted) {
+ ast_log(LOG_DEBUG, "Insert session to the list. count[%d]\n", count);
+ AST_LIST_INSERT_TAIL(&pjsip_sessions, session, next);
+ }
+
+ AST_LIST_UNLOCK(&pjsip_sessions);
+
+ return;
+}
+
+void ast_sip_session_remove(struct ast_sip_session *session)
+{
+ struct ast_sip_session *iter;
+
+ if (!session) {
+ return;
+ }
+
+ AST_LIST_LOCK(&pjsip_sessions);
+
+ AST_LIST_TRAVERSE_SAFE_BEGIN(&pjsip_sessions, iter, next) {
+ if (iter == session) {
+ AST_RWLIST_REMOVE_CURRENT(next);
+ break;
+ }
+ }
+ AST_LIST_TRAVERSE_SAFE_END;
+
+ AST_LIST_UNLOCK(&pjsip_sessions);
+
+ return;
+}
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 4ce35c9..133488b 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -58,6 +58,8 @@
#define DEFAULT_NUM_SESSION_MEDIA 2
/* Some forward declarations */
+static void handle_session_begin(struct ast_sip_session *session);
+static void handle_session_end(struct ast_sip_session *session);
static void handle_incoming_request(struct ast_sip_session *session, pjsip_rx_data *rdata);
static void handle_incoming_response(struct ast_sip_session *session, pjsip_rx_data *rdata,
enum ast_sip_session_response_priority response_priority);
@@ -2104,19 +2106,22 @@
, session->contact ? ast_sorcery_object_get_id(session->contact) : "<none>"
);
+ AST_LIST_LOCK(&session->supplements);
while ((supplement = AST_LIST_REMOVE_HEAD(&session->supplements, next))) {
if (supplement->session_destroy) {
supplement->session_destroy(session);
}
ast_free(supplement);
}
+ AST_LIST_UNLOCK(&session->supplements);
+
+ AST_LIST_HEAD_DESTROY(&session->supplements);
ast_taskprocessor_unreference(session->serializer);
ao2_cleanup(session->datastores);
ast_sip_session_media_state_free(session->active_media_state);
ast_sip_session_media_state_free(session->pending_media_state);
- AST_LIST_HEAD_DESTROY(&session->supplements);
while ((delay = AST_LIST_REMOVE_HEAD(&session->delayed_requests, next))) {
delayed_request_free(delay);
}
@@ -2132,6 +2137,9 @@
pjsip_dlg_dec_session(session->inv_session->dlg, &session_module);
}
+ /* Remove session from the list */
+ ast_sip_session_remove(session);
+
ast_test_suite_event_notify("SESSION_DESTROYED", "Endpoint: %s", endpoint_name);
}
@@ -2211,6 +2219,9 @@
session->endpoint = ao2_bump(endpoint);
+ /* Insert to list */
+ ast_sip_session_insert(session);
+
if (rdata) {
/*
* We must continue using the serializer that the original
@@ -2246,11 +2257,9 @@
ao2_ref(session, -1);
return NULL;
}
- AST_LIST_TRAVERSE(&session->supplements, iter, next) {
- if (iter->session_begin) {
- iter->session_begin(session);
- }
- }
+
+ /* Fire seesion begin handlers */
+ handle_session_begin(session);
/* Avoid unnecessary ref manipulation to return a session */
ret_session = session;
@@ -3135,21 +3144,31 @@
return pj_stristr(&method, message_method) ? PJ_TRUE : PJ_FALSE;
}
-static pj_bool_t has_supplement(const struct ast_sip_session *session, const pjsip_rx_data *rdata)
+static pj_bool_t has_supplement(struct ast_sip_session *session, const pjsip_rx_data *rdata)
{
struct ast_sip_session_supplement *supplement;
struct pjsip_method *method = &rdata->msg_info.msg->line.req.method;
+ int found;
if (!session) {
return PJ_FALSE;
}
+ AST_LIST_LOCK(&session->supplements);
+ found = 0;
AST_LIST_TRAVERSE(&session->supplements, supplement, next) {
if (does_method_match(&method->name, supplement->method)) {
- return PJ_TRUE;
+ found = 1;
+ break;
}
}
- return PJ_FALSE;
+ AST_LIST_UNLOCK(&session->supplements);
+
+ if(!found) {
+ return PJ_FALSE;
+ }
+
+ return PJ_TRUE;
}
/*!
* \brief Called when a new SIP request comes into PJSIP
@@ -3306,13 +3325,56 @@
struct pjsip_request_line req = rdata->msg_info.msg->line.req;
ast_debug(3, "Method is %.*s\n", (int) pj_strlen(&req.method.name), pj_strbuf(&req.method.name));
- AST_LIST_TRAVERSE(&session->supplements, supplement, next) {
+
+ AST_LIST_LOCK(&session->supplements);
+
+ AST_LIST_TRAVERSE_SAFE_BEGIN(&session->supplements, supplement, next) {
if (supplement->incoming_request && does_method_match(&req.method.name, supplement->method)) {
if (supplement->incoming_request(session, rdata)) {
break;
}
}
}
+ AST_LIST_TRAVERSE_SAFE_END;
+
+ AST_LIST_UNLOCK(&session->supplements);
+}
+
+static void handle_session_begin(struct ast_sip_session *session)
+{
+ struct ast_sip_session_supplement *iter;
+
+ ast_log(LOG_DEBUG, "Fired handle_session_begin.\n");
+
+ AST_LIST_LOCK(&session->supplements);
+
+ AST_LIST_TRAVERSE_SAFE_BEGIN(&session->supplements, iter, next) {
+ if (iter->session_begin) {
+ iter->session_begin(session);
+ }
+ }
+ AST_LIST_TRAVERSE_SAFE_END;
+
+ AST_LIST_UNLOCK(&session->supplements);
+}
+
+static void handle_session_end(struct ast_sip_session *session)
+{
+ struct ast_sip_session_supplement *iter;
+
+ ast_log(LOG_DEBUG, "Fired handle_session_end.\n");
+
+ AST_LIST_LOCK(&session->supplements);
+
+ /* Session is dead. Notify the supplements. */
+ AST_LIST_TRAVERSE_SAFE_BEGIN(&session->supplements, iter, next) {
+ if (iter->session_end) {
+ iter->session_end(session);
+ }
+ }
+ AST_LIST_TRAVERSE_SAFE_END;
+
+ AST_LIST_UNLOCK(&session->supplements);
}
static void handle_incoming_response(struct ast_sip_session *session, pjsip_rx_data *rdata,
@@ -3324,7 +3386,9 @@
ast_debug(3, "Response is %d %.*s\n", status.code, (int) pj_strlen(&status.reason),
pj_strbuf(&status.reason));
- AST_LIST_TRAVERSE(&session->supplements, supplement, next) {
+ AST_LIST_LOCK(&session->supplements);
+
+ AST_LIST_TRAVERSE_SAFE_BEGIN(&session->supplements, supplement, next) {
if (!(supplement->response_priority & response_priority)) {
continue;
}
@@ -3332,6 +3396,9 @@
supplement->incoming_response(session, rdata);
}
}
+ AST_LIST_TRAVERSE_SAFE_END;
+
+ AST_LIST_UNLOCK(&session->supplements);
}
static int handle_incoming(struct ast_sip_session *session, pjsip_rx_data *rdata,
@@ -3358,11 +3425,17 @@
ast_sip_message_apply_transport(session->endpoint->transport, tdata);
- AST_LIST_TRAVERSE(&session->supplements, supplement, next) {
+ AST_LIST_LOCK(&session->supplements);
+
+ AST_LIST_TRAVERSE_SAFE_BEGIN(&session->supplements, supplement, next) {
if (supplement->outgoing_request && does_method_match(&req.method.name, supplement->method)) {
supplement->outgoing_request(session, tdata);
}
}
+ AST_LIST_TRAVERSE_SAFE_END;
+
+ AST_LIST_UNLOCK(&session->supplements);
+
}
static void handle_outgoing_response(struct ast_sip_session *session, pjsip_tx_data *tdata)
@@ -3382,11 +3455,17 @@
ast_sip_message_apply_transport(session->endpoint->transport, tdata);
- AST_LIST_TRAVERSE(&session->supplements, supplement, next) {
+ AST_LIST_LOCK(&session->supplements);
+
+ AST_LIST_TRAVERSE_SAFE_BEGIN(&session->supplements, supplement, next) {
if (supplement->outgoing_response && does_method_match(&cseq->method.name, supplement->method)) {
supplement->outgoing_response(session, tdata);
}
}
+ AST_LIST_TRAVERSE_SAFE_END;
+
+ AST_LIST_UNLOCK(&session->supplements);
+
}
static int session_end(void *vsession)
@@ -3398,11 +3477,8 @@
sip_session_defer_termination_stop_timer(session);
/* Session is dead. Notify the supplements. */
- AST_LIST_TRAVERSE(&session->supplements, iter, next) {
- if (iter->session_end) {
- iter->session_end(session);
- }
- }
+ handle_session_end(session);
+
return 0;
}
--
To view, visit https://gerrit.asterisk.org/10617
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5b82be3a75d702cf1933d8d1417f44aa10ad1029
Gerrit-Change-Number: 10617
Gerrit-PatchSet: 1
Gerrit-Owner: sungtae kim <pchero21 at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20181111/b65109fe/attachment-0001.html>
More information about the asterisk-code-review
mailing list