[prev in list] [next in list] [prev in thread] [next in thread]
List: asterisk-dev
Subject: Re: [asterisk-dev] [Code Review] 2562: Avoid unnecessary cleanups during immediate shutdown
From: "svnbot" <reviewboard () asterisk ! org>
Date: 2013-05-30 17:06:05
Message-ID: 20130530170605.17144.53511 () hotblack ! digium ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2562/
-----------------------------------------------------------
(Updated May 30, 2013, 12:06 p.m.)
Status
------
This change has been marked as submitted.
Review request for Asterisk Developers and rmudgett.
Changes
-------
Committed in revision 390122
Repository: Asterisk
Description
-------
This patch addresses issues during immediate shutdowns, where modules
are not unloaded, but Asterisk atexit handlers are run.
In the typical case, this usually isn't a big deal. But the
introduction of the Stasis message bus makes it much more likely for
asynchronous activity to be happening off in some thread during
shutdown.
During an immediate shutdown, Asterisk skips unloading modules. But
while it is processing the atexit handlers, there is a window of time
where some of the core message types have been cleaned up, but the
message bus is still running. Specifically, it's still running
module subscriptions that might be using the core message types. If a
message is received by that subscription in that window, it will
attempt to use a message type that has been cleaned up.
To solve this problem, this patch introduces ast_register_cleanup().
This function operates identically to ast_register_atexit(), except
that cleanup calls are not invoked on an immediate shutdown. All of
the core message type and topic cleanup was moved from atexit handlers
to cleanup handlers.
This ensures that core type and topic cleanup only happens if the
modules that used them are first unloaded.
This patch also changes the ast_assert() when accessing a cleaned up
or uninitialized message type to an error log message. Message type
functions are actually NULL safe across the board, so the assert was a
bit heavy handed. Especially for anyone with DO_CRASH enabled.
Diffs
-----
/trunk/main/stasis.c 389546
/trunk/main/security_events.c 389546
/trunk/main/presencestate.c 389546
/trunk/main/named_acl.c 389546
/trunk/main/devicestate.c 389546
/trunk/main/channel.c 389546
/trunk/main/bridging.c 389546
/trunk/main/asterisk.c 389546
/trunk/main/app.c 389546
/trunk/include/asterisk/stasis_channels.h 389546
/trunk/include/asterisk/stasis_bridging.h 389546
/trunk/include/asterisk/stasis.h 389546
/trunk/include/asterisk/security_events.h 389546
/trunk/include/asterisk.h 389546
/trunk/main/stasis_bridging.c 389546
/trunk/main/stasis_cache.c 389546
/trunk/main/stasis_channels.c 389546
/trunk/main/test.c 389546
Diff: https://reviewboard.asterisk.org/r/2562/diff/
Testing
-------
core stop {now,gracefully} both with and without active channels.
Thanks,
David Lee
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/2562/">https://reviewboard.asterisk.org/r/2562/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#e0e0e0" width="100%" cellpadding="8" style="border: 1px gray \
solid;"> <tr>
<td>
<h1 style="margin-right: 0.2em; padding: 0; font-size: 10pt;">This change has been \
marked as submitted.</h1> </td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for Asterisk Developers and rmudgett.</div>
<div>By David Lee.</div>
<p style="color: grey;"><i>Updated May 30, 2013, 12:06 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Committed in revision 390122</pre> </td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">This patch addresses issues during immediate shutdowns, where modules \
are not unloaded, but Asterisk atexit handlers are run.
In the typical case, this usually isn't a big deal. But the
introduction of the Stasis message bus makes it much more likely for
asynchronous activity to be happening off in some thread during
shutdown.
During an immediate shutdown, Asterisk skips unloading modules. But
while it is processing the atexit handlers, there is a window of time
where some of the core message types have been cleaned up, but the
message bus is still running. Specifically, it's still running
module subscriptions that might be using the core message types. If a
message is received by that subscription in that window, it will
attempt to use a message type that has been cleaned up.
To solve this problem, this patch introduces ast_register_cleanup().
This function operates identically to ast_register_atexit(), except
that cleanup calls are not invoked on an immediate shutdown. All of
the core message type and topic cleanup was moved from atexit handlers
to cleanup handlers.
This ensures that core type and topic cleanup only happens if the
modules that used them are first unloaded.
This patch also changes the ast_assert() when accessing a cleaned up
or uninitialized message type to an error log message. Message type
functions are actually NULL safe across the board, so the assert was a
bit heavy handed. Especially for anyone with DO_CRASH enabled.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">core stop {now,gracefully} both with and without active channels.</pre> \
</td> </tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/main/stasis.c <span style="color: grey">(389546)</span></li>
<li>/trunk/main/security_events.c <span style="color: grey">(389546)</span></li>
<li>/trunk/main/presencestate.c <span style="color: grey">(389546)</span></li>
<li>/trunk/main/named_acl.c <span style="color: grey">(389546)</span></li>
<li>/trunk/main/devicestate.c <span style="color: grey">(389546)</span></li>
<li>/trunk/main/channel.c <span style="color: grey">(389546)</span></li>
<li>/trunk/main/bridging.c <span style="color: grey">(389546)</span></li>
<li>/trunk/main/asterisk.c <span style="color: grey">(389546)</span></li>
<li>/trunk/main/app.c <span style="color: grey">(389546)</span></li>
<li>/trunk/include/asterisk/stasis_channels.h <span style="color: \
grey">(389546)</span></li>
<li>/trunk/include/asterisk/stasis_bridging.h <span style="color: \
grey">(389546)</span></li>
<li>/trunk/include/asterisk/stasis.h <span style="color: grey">(389546)</span></li>
<li>/trunk/include/asterisk/security_events.h <span style="color: \
grey">(389546)</span></li>
<li>/trunk/include/asterisk.h <span style="color: grey">(389546)</span></li>
<li>/trunk/main/stasis_bridging.c <span style="color: grey">(389546)</span></li>
<li>/trunk/main/stasis_cache.c <span style="color: grey">(389546)</span></li>
<li>/trunk/main/stasis_channels.c <span style="color: grey">(389546)</span></li>
<li>/trunk/main/test.c <span style="color: grey">(389546)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2562/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic