[Asterisk-code-review] Stasis: Fix unsafe use of stasis unsubscribe in modules. (asterisk[master])
Corey Farrell
asteriskteam at digium.com
Fri May 22 22:30:23 CDT 2015
Corey Farrell has uploaded a new change for review.
https://gerrit.asterisk.org/527
Change subject: Stasis: Fix unsafe use of stasis_unsubscribe in modules.
......................................................................
Stasis: Fix unsafe use of stasis_unsubscribe in modules.
Many uses of stasis_unsubscribe in modules can be reached through unload.
These have been switched to stasis_unsubscribe_and_join.
Some subscription callbacks do nothing, for these I've created a noop
callback function in stasis.c. This is used by some modules that monitor
MWI topics in order to enable cache, since the callback does not become
invalid after dlclose it is safe to use stasis_unsubscribe on these, even
during module unload.
ASTERISK-25121 #close
Change-Id: Ifc2549fbd8eef7d703c222978e8f452e2972189c
---
M channels/chan_dahdi.c
M channels/chan_iax2.c
M channels/chan_mgcp.c
M channels/chan_sip.c
M channels/chan_skinny.c
M channels/sig_pri.c
M include/asterisk/stasis.h
M main/stasis.c
M res/res_hep_rtcp.c
M res/res_pjsip_mwi.c
M res/res_security_log.c
M res/res_stasis_device_state.c
M res/res_xmpp.c
13 files changed, 38 insertions(+), 35 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/27/527/1
diff --git a/channels/chan_dahdi.c b/channels/chan_dahdi.c
index 2f637dc..fe61309 100644
--- a/channels/chan_dahdi.c
+++ b/channels/chan_dahdi.c
@@ -600,14 +600,6 @@
static int dahdi_sendtext(struct ast_channel *c, const char *text);
-static void mwi_event_cb(void *userdata, struct stasis_subscription *sub, struct stasis_message *msg)
-{
- /* This module does not handle MWI in an event-based manner. However, it
- * subscribes to MWI for each mailbox that is configured so that the core
- * knows that we care about it. Then, chan_dahdi will get the MWI from the
- * event cache instead of checking the mailbox directly. */
-}
-
/*! \brief Avoid the silly dahdi_getevent which ignores a bunch of events */
static inline int dahdi_get_event(int fd)
{
@@ -12593,7 +12585,11 @@
mailbox_specific_topic = ast_mwi_topic(tmp->mailbox);
if (mailbox_specific_topic) {
- tmp->mwi_event_sub = stasis_subscribe_pool(mailbox_specific_topic, mwi_event_cb, NULL);
+ /* This module does not handle MWI in an event-based manner. However, it
+ * subscribes to MWI for each mailbox that is configured so that the core
+ * knows that we care about it. Then, chan_dahdi will get the MWI from the
+ * event cache instead of checking the mailbox directly. */
+ tmp->mwi_event_sub = stasis_subscribe_pool(mailbox_specific_topic, stasis_subscription_cb_noop, NULL);
}
}
#ifdef HAVE_DAHDI_LINEREVERSE_VMWI
diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c
index 0c27d2d..dbad79d 100644
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -1430,13 +1430,6 @@
return is_allowed;
}
-static void mwi_event_cb(void *userdata, struct stasis_subscription *sub, struct stasis_message *msg)
-{
- /* The MWI subscriptions exist just so the core knows we care about those
- * mailboxes. However, we just grab the events out of the cache when it
- * is time to send MWI, since it is only sent with a REGACK. */
-}
-
static void network_change_stasis_subscribe(void)
{
if (!network_change_sub) {
@@ -13010,7 +13003,10 @@
mailbox_specific_topic = ast_mwi_topic(peer->mailbox);
if (mailbox_specific_topic) {
- peer->mwi_event_sub = stasis_subscribe_pool(mailbox_specific_topic, mwi_event_cb, NULL);
+ /* The MWI subscriptions exist just so the core knows we care about those
+ * mailboxes. However, we just grab the events out of the cache when it
+ * is time to send MWI, since it is only sent with a REGACK. */
+ peer->mwi_event_sub = stasis_subscribe_pool(mailbox_specific_topic, stasis_subscription_cb_noop, NULL);
}
}
diff --git a/channels/chan_mgcp.c b/channels/chan_mgcp.c
index 98c6c30..16d3c65 100644
--- a/channels/chan_mgcp.c
+++ b/channels/chan_mgcp.c
@@ -489,14 +489,6 @@
.func_channel_read = acf_channel_read,
};
-static void mwi_event_cb(void *userdata, struct stasis_subscription *sub, struct stasis_message *msg)
-{
- /* This module does not handle MWI in an event-based manner. However, it
- * subscribes to MWI for each mailbox that is configured so that the core
- * knows that we care about it. Then, chan_mgcp will get the MWI from the
- * event cache instead of checking the mailbox directly. */
-}
-
static int has_voicemail(struct mgcp_endpoint *p)
{
int new_msgs;
@@ -4249,7 +4241,11 @@
mailbox_specific_topic = ast_mwi_topic(e->mailbox);
if (mailbox_specific_topic) {
- e->mwi_event_sub = stasis_subscribe_pool(mailbox_specific_topic, mwi_event_cb, NULL);
+ /* This module does not handle MWI in an event-based manner. However, it
+ * subscribes to MWI for each mailbox that is configured so that the core
+ * knows that we care about it. Then, chan_mgcp will get the MWI from the
+ * event cache instead of checking the mailbox directly. */
+ e->mwi_event_sub = stasis_subscribe_pool(mailbox_specific_topic, stasis_subscription_cb_noop, NULL);
}
}
snprintf(e->rqnt_ident, sizeof(e->rqnt_ident), "%08lx", (unsigned long)ast_random());
diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 9eeb75f..7c4c8a6 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -5008,7 +5008,7 @@
static void destroy_mailbox(struct sip_mailbox *mailbox)
{
if (mailbox->event_sub) {
- mailbox->event_sub = stasis_unsubscribe(mailbox->event_sub);
+ mailbox->event_sub = stasis_unsubscribe_and_join(mailbox->event_sub);
}
ast_free(mailbox);
}
diff --git a/channels/chan_skinny.c b/channels/chan_skinny.c
index 47c7352..03da0e0 100644
--- a/channels/chan_skinny.c
+++ b/channels/chan_skinny.c
@@ -8756,7 +8756,7 @@
skinny_unlocksub(sub);
}
if (l->mwi_event_sub) {
- l->mwi_event_sub = stasis_unsubscribe(l->mwi_event_sub);
+ l->mwi_event_sub = stasis_unsubscribe_and_join(l->mwi_event_sub);
}
ast_mutex_unlock(&l->lock);
unregister_exten(l);
diff --git a/channels/sig_pri.c b/channels/sig_pri.c
index 71e7e23..b009c45 100644
--- a/channels/sig_pri.c
+++ b/channels/sig_pri.c
@@ -8978,7 +8978,7 @@
#if defined(HAVE_PRI_MWI)
for (idx = 0; idx < ARRAY_LEN(pri->mbox); ++idx) {
if (pri->mbox[idx].sub) {
- pri->mbox[idx].sub = stasis_unsubscribe(pri->mbox[idx].sub);
+ pri->mbox[idx].sub = stasis_unsubscribe_and_join(pri->mbox[idx].sub);
}
}
#endif /* defined(HAVE_PRI_MWI) */
diff --git a/include/asterisk/stasis.h b/include/asterisk/stasis.h
index aa681e1..69b2d0f 100644
--- a/include/asterisk/stasis.h
+++ b/include/asterisk/stasis.h
@@ -511,6 +511,17 @@
typedef void (*stasis_subscription_cb)(void *data, struct stasis_subscription *sub, struct stasis_message *message);
/*!
+ * \brief Stasis subscription callback function that does nothing.
+ *
+ * \note This callback should be used for events are not directly processed, but need
+ * to be generated so data can be retrieved from cache later. Subscriptions with this
+ * callback can be released with \ref stasis_unsubscribe, even during module unload.
+ *
+ * \since 13.5
+ */
+void stasis_subscription_cb_noop(void *data, struct stasis_subscription *sub, struct stasis_message *message);
+
+/*!
* \brief Create a subscription.
*
* In addition to being AO2 managed memory (requiring an ao2_cleanup() to free
diff --git a/main/stasis.c b/main/stasis.c
index 6a59265..e168ce9 100644
--- a/main/stasis.c
+++ b/main/stasis.c
@@ -444,6 +444,10 @@
static void send_subscription_subscribe(struct stasis_topic *topic, struct stasis_subscription *sub);
static void send_subscription_unsubscribe(struct stasis_topic *topic, struct stasis_subscription *sub);
+void stasis_subscription_cb_noop(void *data, struct stasis_subscription *sub, struct stasis_message *message)
+{
+}
+
struct stasis_subscription *internal_stasis_subscribe(
struct stasis_topic *topic,
stasis_subscription_cb callback,
diff --git a/res/res_hep_rtcp.c b/res/res_hep_rtcp.c
index 352262b..25aed15 100644
--- a/res/res_hep_rtcp.c
+++ b/res/res_hep_rtcp.c
@@ -131,7 +131,7 @@
static int unload_module(void)
{
if (stasis_rtp_subscription) {
- stasis_rtp_subscription = stasis_unsubscribe(stasis_rtp_subscription);
+ stasis_rtp_subscription = stasis_unsubscribe_and_join(stasis_rtp_subscription);
}
return 0;
diff --git a/res/res_pjsip_mwi.c b/res/res_pjsip_mwi.c
index 8fefc3a..371f3ab 100644
--- a/res/res_pjsip_mwi.c
+++ b/res/res_pjsip_mwi.c
@@ -471,7 +471,7 @@
struct mwi_stasis_subscription *mwi_stasis = obj;
if (mwi_stasis->stasis_sub) {
ast_debug(3, "Removing stasis subscription to mailbox %s\n", mwi_stasis->mailbox);
- mwi_stasis->stasis_sub = stasis_unsubscribe(mwi_stasis->stasis_sub);
+ mwi_stasis->stasis_sub = stasis_unsubscribe_and_join(mwi_stasis->stasis_sub);
}
return CMP_MATCH;
}
diff --git a/res/res_security_log.c b/res/res_security_log.c
index d32b32c..94a78d8 100644
--- a/res/res_security_log.c
+++ b/res/res_security_log.c
@@ -152,7 +152,7 @@
static int unload_module(void)
{
if (security_stasis_sub) {
- security_stasis_sub = stasis_unsubscribe(security_stasis_sub);
+ security_stasis_sub = stasis_unsubscribe_and_join(security_stasis_sub);
}
ast_logger_unregister_level(LOG_SECURITY_NAME);
diff --git a/res/res_stasis_device_state.c b/res/res_stasis_device_state.c
index 1d135fe..7f7c513 100644
--- a/res/res_stasis_device_state.c
+++ b/res/res_stasis_device_state.c
@@ -105,7 +105,7 @@
static void device_state_subscription_destroy(void *obj)
{
struct device_state_subscription *sub = obj;
- sub->sub = stasis_unsubscribe(sub->sub);
+ sub->sub = stasis_unsubscribe_and_join(sub->sub);
ast_string_field_free_memory(sub);
}
diff --git a/res/res_xmpp.c b/res/res_xmpp.c
index 5a3670e..2a087b0 100644
--- a/res/res_xmpp.c
+++ b/res/res_xmpp.c
@@ -3568,12 +3568,12 @@
}
if (client->mwi_sub) {
- client->mwi_sub = stasis_unsubscribe(client->mwi_sub);
+ client->mwi_sub = stasis_unsubscribe_and_join(client->mwi_sub);
xmpp_pubsub_unsubscribe(client, "message_waiting");
}
if (client->device_state_sub) {
- client->device_state_sub = stasis_unsubscribe(client->device_state_sub);
+ client->device_state_sub = stasis_unsubscribe_and_join(client->device_state_sub);
xmpp_pubsub_unsubscribe(client, "device_state");
}
--
To view, visit https://gerrit.asterisk.org/527
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifc2549fbd8eef7d703c222978e8f452e2972189c
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Corey Farrell <git at cfware.com>
More information about the asterisk-code-review
mailing list