[asterisk-commits] twilson: trunk r367309 - in /trunk: ./ include/asterisk/ main/
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Tue May 22 12:29:22 CDT 2012
Author: twilson
Date: Tue May 22 12:29:12 2012
New Revision: 367309
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=367309
Log:
Fix race condition for CEL LINKEDID_END event
This patch fixes to situations that could cause the CEL LINKEDID_END event to
be missed.
1) During a core stop gracefully, modules are unloaded when ast_active_channels
== 0. The LINKDEDID_END event fires during the channel destructor. This means
that occasionally, the cel_* module will be unloaded before the channel is
destroyed. It seemed generally useful to wait until the refcount of all
channels == 0 before unloading, so I added a channel counter and used it in the
shutdown code.
2) During a masquerade, ast_channel_change_linkedid is called. It calls
ast_cel_check_retire_linkedid which unrefs the linkedid in the linkedids
container in cel.c. It didn't ref the new linkedid. Now it does.
Review: https://reviewboard.asterisk.org/r/1900/
........
Merged revisions 367292 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........
Merged revisions 367299 from http://svn.asterisk.org/svn/asterisk/branches/10
Modified:
trunk/ (props changed)
trunk/include/asterisk/cel.h
trunk/include/asterisk/channel.h
trunk/main/asterisk.c
trunk/main/cel.c
trunk/main/channel.c
trunk/main/channel_internal_api.c
Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-10-merged' - no diff available.
Modified: trunk/include/asterisk/cel.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/cel.h?view=diff&rev=367309&r1=367308&r2=367309
==============================================================================
--- trunk/include/asterisk/cel.h (original)
+++ trunk/include/asterisk/cel.h Tue May 22 12:29:12 2012
@@ -188,6 +188,15 @@
void ast_cel_check_retire_linkedid(struct ast_channel *chan);
/*!
+ * \brief Inform CEL that a new linkedid is being used
+ * \since 11
+ *
+ * \retval -1 error
+ * \retval 0 success
+ */
+int ast_cel_linkedid_ref(const char *linkedid);
+
+/*!
* \brief Create a fake channel from data in a CEL event
*
* \note
Modified: trunk/include/asterisk/channel.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/channel.h?view=diff&rev=367309&r1=367308&r2=367309
==============================================================================
--- trunk/include/asterisk/channel.h (original)
+++ trunk/include/asterisk/channel.h Tue May 22 12:29:12 2012
@@ -2104,8 +2104,11 @@
/*! Cancels an existing shutdown and returns to normal operation */
void ast_cancel_shutdown(void);
-/*! \return number of active/allocated channels */
+/*! \return number of channels available for lookup */
int ast_active_channels(void);
+
+/*! \return the number of channels not yet destroyed */
+int ast_undestroyed_channels(void);
/*! \return non-zero if Asterisk is being shut down */
int ast_shutting_down(void);
Modified: trunk/main/asterisk.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/asterisk.c?view=diff&rev=367309&r1=367308&r2=367309
==============================================================================
--- trunk/main/asterisk.c (original)
+++ trunk/main/asterisk.c Tue May 22 12:29:12 2012
@@ -1704,7 +1704,7 @@
for (;;) {
time(&e);
/* Wait up to 15 seconds for all channels to go away */
- if ((e - s) > 15 || !ast_active_channels() || shuttingdown != niceness) {
+ if ((e - s) > 15 || !ast_undestroyed_channels() || shuttingdown != niceness) {
break;
}
/* Sleep 1/10 of a second */
@@ -1718,7 +1718,7 @@
ast_verb(0, "Waiting for inactivity to perform %s...\n", restart ? "restart" : "halt");
}
for (;;) {
- if (!ast_active_channels() || shuttingdown != niceness) {
+ if (!ast_undestroyed_channels() || shuttingdown != niceness) {
break;
}
sleep(1);
Modified: trunk/main/cel.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/cel.c?view=diff&rev=367309&r1=367308&r2=367309
==============================================================================
--- trunk/main/cel.c (original)
+++ trunk/main/cel.c Tue May 22 12:29:12 2012
@@ -470,6 +470,31 @@
return tchan;
}
+int ast_cel_linkedid_ref(const char *linkedid)
+{
+ char *lid;
+
+ if (ast_strlen_zero(linkedid)) {
+ ast_log(LOG_ERROR, "The linkedid should never be empty\n");
+ return -1;
+ }
+
+ if (!(lid = ao2_find(linkedids, (void *) linkedid, OBJ_POINTER))) {
+ if (!(lid = ao2_alloc(strlen(linkedid) + 1, NULL))) {
+ return -1;
+ }
+ strcpy(lid, linkedid);
+ if (!ao2_link(linkedids, lid)) {
+ ao2_ref(lid, -1);
+ return -1;
+ }
+ /* Leave both the link and the alloc refs to show a count of 1 + the link */
+ }
+ /* If we've found, go ahead and keep the ref to increment count of how many channels
+ * have this linkedid. We'll clean it up in check_retire */
+ return 0;
+}
+
int ast_cel_report_event(struct ast_channel *chan, enum ast_cel_event_type event_type,
const char *userdefevname, const char *extra, struct ast_channel *peer2)
{
@@ -487,22 +512,10 @@
/* Record the linkedid of new channels if we are tracking LINKEDID_END even if we aren't
* reporting on CHANNEL_START so we can track when to send LINKEDID_END */
if (cel_enabled && ast_cel_track_event(AST_CEL_LINKEDID_END) && event_type == AST_CEL_CHANNEL_START && linkedid) {
- char *lid;
- if (!(lid = ao2_find(linkedids, (void *) linkedid, OBJ_POINTER))) {
- if (!(lid = ao2_alloc(strlen(linkedid) + 1, NULL))) {
- ast_mutex_unlock(&reload_lock);
- return -1;
- }
- strcpy(lid, linkedid);
- if (!ao2_link(linkedids, lid)) {
- ao2_ref(lid, -1);
- ast_mutex_unlock(&reload_lock);
- return -1;
- }
- /* Leave both the link and the alloc refs to show a count of 1 + the link */
- }
- /* If we've found, go ahead and keep the ref to increment count of how many channels
- * have this linkedid. We'll clean it up in check_retire */
+ if (ast_cel_linkedid_ref(linkedid)) {
+ ast_mutex_unlock(&reload_lock);
+ return -1;
+ }
}
if (!cel_enabled || !ast_cel_track_event(event_type)) {
Modified: trunk/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/channel.c?view=diff&rev=367309&r1=367308&r2=367309
==============================================================================
--- trunk/main/channel.c (original)
+++ trunk/main/channel.c Tue May 22 12:29:12 2012
@@ -94,6 +94,7 @@
static int shutting_down;
static int uniqueint;
+static int chancount;
unsigned long global_fin, global_fout;
@@ -645,6 +646,11 @@
return channels ? ao2_container_count(channels) : 0;
}
+int ast_undestroyed_channels(void)
+{
+ return ast_atomic_fetchadd_int(&chancount, 0);
+}
+
/*! \brief Cancel a shutdown in progress */
void ast_cancel_shutdown(void)
{
@@ -1095,6 +1101,7 @@
ast_cdr_init(ast_channel_cdr(tmp), tmp);
ast_cdr_start(ast_channel_cdr(tmp));
+ ast_atomic_fetchadd_int(&chancount, +1);
ast_cel_report_event(tmp, AST_CEL_CHANNEL_START, NULL, NULL, NULL);
headp = ast_channel_varshead(tmp);
@@ -2302,6 +2309,7 @@
if (callid) {
ast_callid_unref(callid);
}
+ ast_atomic_fetchadd_int(&chancount, -1);
}
/*! \brief Free a dummy channel structure */
@@ -6357,14 +6365,16 @@
* see if the channel's old linkedid is now being retired */
static void ast_channel_change_linkedid(struct ast_channel *chan, const char *linkedid)
{
+ ast_assert(linkedid != NULL);
/* if the linkedid for this channel is being changed from something, check... */
- if (!ast_strlen_zero(ast_channel_linkedid(chan)) && 0 != strcmp(ast_channel_linkedid(chan), linkedid)) {
- ast_cel_check_retire_linkedid(chan);
- }
-
+ if (ast_channel_linkedid(chan) && !strcmp(ast_channel_linkedid(chan), linkedid)) {
+ return;
+ }
+
+ ast_cel_check_retire_linkedid(chan);
ast_channel_linkedid_set(chan, linkedid);
-}
-
+ ast_cel_linkedid_ref(linkedid);
+}
/*!
\brief Propagate the oldest linkedid between associated channels
Modified: trunk/main/channel_internal_api.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/channel_internal_api.c?view=diff&rev=367309&r1=367308&r2=367309
==============================================================================
--- trunk/main/channel_internal_api.c (original)
+++ trunk/main/channel_internal_api.c Tue May 22 12:29:12 2012
@@ -423,7 +423,6 @@
DEFINE_STRINGFIELD_SETTERS_FOR(userfield);
DEFINE_STRINGFIELD_SETTERS_FOR(call_forward);
DEFINE_STRINGFIELD_SETTERS_FOR(uniqueid);
-DEFINE_STRINGFIELD_SETTERS_FOR(linkedid);
DEFINE_STRINGFIELD_SETTERS_FOR(parkinglot);
DEFINE_STRINGFIELD_SETTERS_FOR(hangupsource);
DEFINE_STRINGFIELD_SETTERS_FOR(dialcontext);
@@ -445,6 +444,12 @@
DEFINE_STRINGFIELD_GETTER_FOR(parkinglot);
DEFINE_STRINGFIELD_GETTER_FOR(hangupsource);
DEFINE_STRINGFIELD_GETTER_FOR(dialcontext);
+
+void ast_channel_linkedid_set(struct ast_channel *chan, const char *value)
+{
+ ast_assert(!ast_strlen_zero(value));
+ ast_string_field_set(chan, linkedid, value);
+}
const char *ast_channel_appl(const struct ast_channel *chan)
{
More information about the asterisk-commits
mailing list