[asterisk-dev] [Code Review] 2562: Avoid unnecessary cleanups during immediate shutdown

[opticron]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2562/#review8722
-----------------------------------------------------------

Ship it!


Ship It!

- opticron


On May 23, 2013, 9:58 a.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2562/
> -----------------------------------------------------------
> 
> (Updated May 23, 2013, 9:58 a.m.)
> 
> 
> Review request for Asterisk Developers and rmudgett.
> 
> 
> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130529/0d2dd359/attachment-0001.htm>