[Asterisk-code-review] res pjsip: Patch for res pjsip * module load/reload crash (asterisk[master])
George Joseph
asteriskteam at digium.com
Tue Dec 18 10:42:50 CST 2018
George Joseph has submitted this change and it was merged. ( 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_message_filter.c
M res/res_pjsip/pjsip_session.c
M res/res_pjsip_session.c
4 files changed, 94 insertions(+), 22 deletions(-)
Approvals:
Kevin Harwell: Looks good to me, but someone else must approve
Benjamin Keith Ford: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved; Approved for Submit
diff --git a/include/asterisk/res_pjsip_session.h b/include/asterisk/res_pjsip_session.h
index ccee8c9..34f4783 100644
--- a/include/asterisk/res_pjsip_session.h
+++ b/include/asterisk/res_pjsip_session.h
@@ -258,6 +258,8 @@
* processing to incoming and outgoing SIP requests and responses
*/
struct ast_sip_session_supplement {
+ /*! Reference module info */
+ struct ast_module *module;
/*! Method on which to call the callbacks. If NULL, call on all methods */
const char *method;
/*! Priority for this supplement. Lower numbers are visited before higher numbers */
@@ -580,9 +582,13 @@
* set channel data based on headers in an incoming message. Similarly,
* a module could reject an incoming request if desired.
*
+ * \param module Referenced module(NULL safe)
* \param supplement The supplement to register
*/
-void ast_sip_session_register_supplement(struct ast_sip_session_supplement *supplement);
+void ast_sip_session_register_supplement_with_module(struct ast_module *module, struct ast_sip_session_supplement *supplement);
+
+#define ast_sip_session_register_supplement(supplement) \
+ ast_sip_session_register_supplement_with_module(AST_MODULE_SELF, supplement)
/*!
* \brief Unregister a an supplement to SIP session processing
@@ -599,6 +605,13 @@
int ast_sip_session_add_supplements(struct ast_sip_session *session);
/*!
+ * \brief Remove supplements from a SIP session
+ *
+ * \param session The session to remove
+ */
+void ast_sip_session_remove_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/pjsip_message_filter.c b/res/res_pjsip/pjsip_message_filter.c
index f948c44..4b0f60d 100644
--- a/res/res_pjsip/pjsip_message_filter.c
+++ b/res/res_pjsip/pjsip_message_filter.c
@@ -24,6 +24,7 @@
#include "asterisk/res_pjsip.h"
#include "asterisk/res_pjsip_session.h"
#include "include/res_pjsip_private.h"
+#include "asterisk/module.h"
#define MOD_DATA_RESTRICTIONS "restrictions"
diff --git a/res/res_pjsip/pjsip_session.c b/res/res_pjsip/pjsip_session.c
index f3f3a4d..7dc6221 100644
--- a/res/res_pjsip/pjsip_session.c
+++ b/res/res_pjsip/pjsip_session.c
@@ -32,12 +32,16 @@
AST_RWLIST_HEAD_STATIC(session_supplements, ast_sip_session_supplement);
-void ast_sip_session_register_supplement(struct ast_sip_session_supplement *supplement)
+void ast_sip_session_register_supplement_with_module(struct ast_module *module, 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);
+ ast_assert(supplement != NULL);
+
+ supplement->module = module;
+
if (!supplement->response_priority) {
supplement->response_priority = AST_SIP_SESSION_BEFORE_MEDIA;
}
@@ -94,8 +98,34 @@
if (!copy) {
return -1;
}
+
+ /* referenced session created. increasing module reference. */
+ ast_module_ref(copy->module);
+
AST_LIST_INSERT_TAIL(&session->supplements, copy, next);
}
return 0;
}
+
+void ast_sip_session_remove_supplements(struct ast_sip_session *session)
+{
+ struct ast_sip_session_supplement *iter;
+
+ if (!session) {
+ return;
+ }
+
+ /* free the supplements */
+ while ((iter = AST_LIST_REMOVE_HEAD(&session->supplements, next))) {
+ if (iter->module) {
+ /* referenced session closed. decreasing modue reference. */
+ ast_module_unref(iter->module);
+ }
+
+ ast_free(iter);
+ }
+
+ return;
+}
+
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 94240d3..99e0ca4 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -58,6 +58,9 @@
#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_session_destroy(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);
@@ -2088,7 +2091,6 @@
static void session_destructor(void *obj)
{
struct ast_sip_session *session = obj;
- struct ast_sip_session_supplement *supplement;
struct ast_sip_session_delayed_request *delay;
const char *endpoint_name = session->endpoint ?
ast_sorcery_object_get_id(session->endpoint) : "<none>";
@@ -2104,19 +2106,18 @@
, session->contact ? ast_sorcery_object_get_id(session->contact) : "<none>"
);
- while ((supplement = AST_LIST_REMOVE_HEAD(&session->supplements, next))) {
- if (supplement->session_destroy) {
- supplement->session_destroy(session);
- }
- ast_free(supplement);
- }
+ /* fire session destroy handler */
+ handle_session_destroy(session);
+
+ /* remove all registered supplements */
+ ast_sip_session_remove_supplements(session);
+ 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);
}
@@ -2165,7 +2166,6 @@
{
RAII_VAR(struct ast_sip_session *, session, NULL, ao2_cleanup);
struct ast_sip_session *ret_session;
- struct ast_sip_session_supplement *iter;
int dsp_features = 0;
session = ao2_alloc(sizeof(*session), session_destructor);
@@ -2247,11 +2247,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;
@@ -3316,6 +3314,40 @@
}
}
+static void handle_session_begin(struct ast_sip_session *session)
+{
+ struct ast_sip_session_supplement *iter;
+
+ AST_LIST_TRAVERSE(&session->supplements, iter, next) {
+ if (iter->session_begin) {
+ iter->session_begin(session);
+ }
+ }
+}
+
+static void handle_session_destroy(struct ast_sip_session *session)
+{
+ struct ast_sip_session_supplement *iter;
+
+ AST_LIST_TRAVERSE(&session->supplements, iter, next) {
+ if (iter->session_destroy) {
+ iter->session_destroy(session);
+ }
+ }
+}
+
+static void handle_session_end(struct ast_sip_session *session)
+{
+ struct ast_sip_session_supplement *iter;
+
+ /* Session is dead. Notify the supplements. */
+ AST_LIST_TRAVERSE(&session->supplements, iter, next) {
+ if (iter->session_end) {
+ iter->session_end(session);
+ }
+ }
+}
+
static void handle_incoming_response(struct ast_sip_session *session, pjsip_rx_data *rdata,
enum ast_sip_session_response_priority response_priority)
{
@@ -3393,17 +3425,13 @@
static int session_end(void *vsession)
{
struct ast_sip_session *session = vsession;
- struct ast_sip_session_supplement *iter;
/* Stop the scheduled termination */
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: merged
Gerrit-Change-Id: I5b82be3a75d702cf1933d8d1417f44aa10ad1029
Gerrit-Change-Number: 10617
Gerrit-PatchSet: 11
Gerrit-Owner: sungtae kim <pchero21 at gmail.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.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: sungtae kim <pchero21 at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20181218/efc62b38/attachment-0001.html>
More information about the asterisk-code-review
mailing list