rtp_engine and stun: call ast_register_atexit instead of ast_register_cleanup

[gtjoseph]

rtp_engine.c and stun.c were calling ast_register_cleanup which is skipped if any loadable module can't be cleanly unloaded when asterisk shuts down. Since this will always be the case, their cleanup functions never get run. In a practical sense this makes no difference since asterisk is shutting down but if you're in development mode and trying to use the leak sanitizer, the leaks from both of those modules clutter up the output.

[gtjoseph]

cherry-pick-to: 18 cherry-pick-to: 20 cherry-pick-to: 21 cherry-pick-to: certified/18.9 cherry-pick-to: certified/20.7

[InterLinked1]

rtp_engine.c and stun.c were calling ast_register_cleanup which is skipped if any loadable module can't be cleanly unloaded when asterisk shuts down. Since this will always be the case, their cleanup functions never get run. Core modules should always call ast_register_atexit for this reason.

The commit makes sense, but this change seems to contradict the documentation slightly: https://github.com/asterisk/asterisk/blob/52881ef6e513e099d7a1b0259551765ac075f90a/include/asterisk.h#L112

\note This function should be rarely used in situations where * something must be shutdown to avoid corruption, excessive data * loss, or when external programs must be stopped. All other * cleanup in the core should use ast_register_cleanup.

At least, RTP and STUN don't strike me as situations where they need to be shut down for those reasons.

Would it make sense to have a mode that really cleans everything up, for use with sanitizers, valgrind, etc? Or am I missing something here?

[gtjoseph]

Would it make sense to have a mode that really cleans everything up, for use with sanitizers, valgrind, etc? Or am I missing something here?

No because in many cases if a module fails to unload and you "clean up everything" then a segfault is likely to occur if a module is still running. If that happens the sanitizers and valgrind will be useless. My assertion that "Core modules should always call ast_register_atexit for this reason." is actually incorrect and I'm going to remove that statement.

It just so happens that there are only 4 places where there's a memory cleanup issue...

• The editline history in asterisk.c for which we call the proper cleanups but for some reason they're still leaving 2 allocations.
• test_strings.c leaves 1 allocation but who cares? (I'll fix this later).
• rtp_engine.c
• stun.c

The issue with rtp_engine and stun are only the result of not cleaning up their calls to ast_debug_category_register so those are not going to affect any loaded modules which is why it's safe to use the atexit calls which will run even if any modules fail to unload.

[github-actions[bot]]

Successfully merged to branch master and cherry-picked to ["18","20","21","certified/18.9","certified/20.7"]