core: Add cache_media_frames debugging option.

The media frame cache gets in the way of finding use after free errors of
media frames.  Tools like valgrind and MALLOC_DEBUG don't know when a
frame is released because it gets put into the cache instead of being
freed.

* Added the "cache_media_frames" option to asterisk.conf.  Disabling the
option helps track down media frame mismanagement when using valgrind or
MALLOC_DEBUG.  The cache gets in the way of determining if the frame is
used after free and who freed it.  NOTE: This option has no effect when
Asterisk is compiled with the LOW_MEMORY compile time option enabled
because the cache code does not exist.

To disable the media frame cache simply disable the cache_media_frames
option in asterisk.conf and restart Asterisk.

Sample asterisk.conf setting:
[options]
cache_media_frames=no

ASTERISK-27413

Change-Id: I0ab2ce0f4547cccf2eb214901835c2d951b78c00

diff --git a/CHANGES b/CHANGES
index 74ebf64f39dab387d0ba3994220cd1dfa24438f9..7dd1aac27e9b7008d01ac1fe8aa2b711efa6beb9 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -12,6 +12,15 @@
 --- Functionality changes from Asterisk 13.18.0 to Asterisk 13.19.0 ----------
 ------------------------------------------------------------------------------
 
+Core
+------------------
+ * Added the "cache_media_frames" option to asterisk.conf.  Disabling the option
+   helps track down media frame mismanagement when using valgrind or
+   MALLOC_DEBUG.  The cache gets in the way of determining if the frame is
+   used after free and who freed it.  NOTE: This option has no effect when
+   Asterisk is compiled with the LOW_MEMORY compile time option enabled because
+   the cache code does not exist.
+
 res_pjsip
 ------------------
  * The "identify_by" on endpoints can now be set to "ip" to restrict an endpoint

diff --git a/channels/iax2/parser.c b/channels/iax2/parser.c
index 2a3eabf4128e563c896ae3d566a433c7bae51854..09c13238dae77bb34f2201923f7f76496c1344c9 100644 (file)
--- a/channels/iax2/parser.c
+++ b/channels/iax2/parser.c
@@ -1298,7 +1298,9 @@ void iax_frame_free(struct iax_frame *fr)
        ast_atomic_fetchadd_int(&frames, -1);
 
 #if !defined(LOW_MEMORY)
-       if (!fr->cacheable || !(iax_frames = ast_threadstorage_get(&frame_cache, sizeof(*iax_frames)))) {
+       if (!fr->cacheable
+               || !ast_opt_cache_media_frames
+               || !(iax_frames = ast_threadstorage_get(&frame_cache, sizeof(*iax_frames)))) {
                ast_free(fr);
                return;
        }

diff --git a/configs/samples/asterisk.conf.sample b/configs/samples/asterisk.conf.sample
index 38cee250278aea1645ccdc57b0455f274be7569a..e57ee10e09afe37f447bf64b05318b37cb7dda14 100644 (file)
--- a/configs/samples/asterisk.conf.sample
+++ b/configs/samples/asterisk.conf.sample
@@ -43,6 +43,15 @@ astsbindir => /usr/sbin
 ;minmemfree = 1                        ; In MBs, Asterisk stops accepting new calls if
                                ; the amount of free memory falls below this
                                ; watermark.
+;cache_media_frames = yes      ; Cache media frames for performance
+                               ; Disable this option to help track down media frame
+                               ; mismanagement when using valgrind or MALLOC_DEBUG.
+                               ; The cache gets in the way of determining if the
+                               ; frame is used after being freed and who freed it.
+                               ; NOTE: This option has no effect when Asterisk is
+                               ; compiled with the LOW_MEMORY compile time option
+                               ; enabled because the cache code does not exist.
+                               ; Default yes
 ;cache_record_files = yes      ; Cache recorded sound files to another
                                ; directory during recording.
 ;record_cache_dir = /tmp       ; Specify cache directory (used in conjunction

diff --git a/include/asterisk/options.h b/include/asterisk/options.h
index 950764e15ce5297226594bf093a0aba6c8ebd32f..c64de2fb19800cd41b6fcc6e13248fce6c3ead0d 100644 (file)
--- a/include/asterisk/options.h
+++ b/include/asterisk/options.h
@@ -76,6 +76,8 @@ enum ast_option_flags {
        AST_OPT_FLAG_DONT_WARN = (1 << 18),
        /*! End CDRs before the 'h' extension */
        AST_OPT_FLAG_END_CDR_BEFORE_H_EXTEN = (1 << 19),
+       /*! Cache media frames for performance */
+       AST_OPT_FLAG_CACHE_MEDIA_FRAMES = (1 << 20),
        /*! Always fork, even if verbose or debug settings are non-zero */
        AST_OPT_FLAG_ALWAYS_FORK = (1 << 21),
        /*! Disable log/verbose output to remote consoles */
@@ -99,7 +101,7 @@ enum ast_option_flags {
 };
 
 /*! These are the options that set by default when Asterisk starts */
-#define AST_DEFAULT_OPTIONS AST_OPT_FLAG_TRANSCODE_VIA_SLIN
+#define AST_DEFAULT_OPTIONS (AST_OPT_FLAG_TRANSCODE_VIA_SLIN | AST_OPT_FLAG_CACHE_MEDIA_FRAMES)
 
 #define ast_opt_exec_includes          ast_test_flag(&ast_options, AST_OPT_FLAG_EXEC_INCLUDES)
 #define ast_opt_no_fork                        ast_test_flag(&ast_options, AST_OPT_FLAG_NO_FORK)
@@ -116,6 +118,7 @@ enum ast_option_flags {
 #define ast_opt_stdexten_macro         ast_test_flag(&ast_options, AST_OPT_FLAG_STDEXTEN_MACRO)
 #define ast_opt_dump_core              ast_test_flag(&ast_options, AST_OPT_FLAG_DUMP_CORE)
 #define ast_opt_cache_record_files     ast_test_flag(&ast_options, AST_OPT_FLAG_CACHE_RECORD_FILES)
+#define ast_opt_cache_media_frames     ast_test_flag(&ast_options, AST_OPT_FLAG_CACHE_MEDIA_FRAMES)
 #define ast_opt_timestamp              ast_test_flag(&ast_options, AST_OPT_FLAG_TIMESTAMP)
 #define ast_opt_override_config                ast_test_flag(&ast_options, AST_OPT_FLAG_OVERRIDE_CONFIG)
 #define ast_opt_reconnect              ast_test_flag(&ast_options, AST_OPT_FLAG_RECONNECT)

diff --git a/main/asterisk.c b/main/asterisk.c
index 8cbbe9318278b9b370fcc87ba35898d56ffec5cc..0eaf8dd0258dabaa86fd217d54f0a6d0671dd1b4 100644 (file)
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -676,6 +676,9 @@ static char *handle_show_settings(struct ast_cli_entry *e, int cmd, struct ast_c
        ast_cli(a->fd, "  Transmit silence during rec: %s\n", ast_test_flag(&ast_options, AST_OPT_FLAG_TRANSMIT_SILENCE) ? "Enabled" : "Disabled");
        ast_cli(a->fd, "  Generic PLC:                 %s\n", ast_test_flag(&ast_options, AST_OPT_FLAG_GENERIC_PLC) ? "Enabled" : "Disabled");
        ast_cli(a->fd, "  Min DTMF duration::          %u\n", option_dtmfminduration);
+#if !defined(LOW_MEMORY)
+       ast_cli(a->fd, "  Cache media frames:          %s\n", ast_opt_cache_media_frames ? "Enabled" : "Disabled");
+#endif
 
        if (ast_option_rtpptdynamic == AST_RTP_PT_LAST_REASSIGN) {
                ast_cli(a->fd, "  RTP dynamic payload types:   %u,%u-%u\n",
@@ -3827,6 +3830,11 @@ static void ast_readconfig(void)
                /* Cache recorded sound files to another directory during recording */
                } else if (!strcasecmp(v->name, "cache_record_files")) {
                        ast_set2_flag(&ast_options, ast_true(v->value), AST_OPT_FLAG_CACHE_RECORD_FILES);
+#if !defined(LOW_MEMORY)
+               /* Cache media frames for performance */
+               } else if (!strcasecmp(v->name, "cache_media_frames")) {
+                       ast_set2_flag(&ast_options, ast_true(v->value), AST_OPT_FLAG_CACHE_MEDIA_FRAMES);
+#endif
                /* Specify cache directory */
                }  else if (!strcasecmp(v->name, "record_cache_dir")) {
                        ast_copy_string(record_cache_dir, v->value, AST_CACHE_DIR_LEN);

diff --git a/main/frame.c b/main/frame.c
index 4261b04dd26439c6fbc50b1582a2f587898c248d..db2b96743791cc64a684a6b972514da1ee9224ab 100644 (file)
--- a/main/frame.c
+++ b/main/frame.c
@@ -122,14 +122,18 @@ static void __frame_free(struct ast_frame *fr, int cache)
                return;
 
 #if !defined(LOW_MEMORY)
-       if (cache && fr->mallocd == AST_MALLOCD_HDR) {
+       if (fr->mallocd == AST_MALLOCD_HDR
+               && cache
+               && ast_opt_cache_media_frames) {
                /* Cool, only the header is malloc'd, let's just cache those for now
                 * to keep things simple... */
                struct ast_frame_cache *frames;
-               if ((frames = ast_threadstorage_get(&frame_cache, sizeof(*frames))) &&
-                   (frames->size < FRAME_CACHE_MAX_SIZE)) {
-                       if ((fr->frametype == AST_FRAME_VOICE) || (fr->frametype == AST_FRAME_VIDEO) ||
-                               (fr->frametype == AST_FRAME_IMAGE)) {
+
+               frames = ast_threadstorage_get(&frame_cache, sizeof(*frames));
+               if (frames && frames->size < FRAME_CACHE_MAX_SIZE) {
+                       if (fr->frametype == AST_FRAME_VOICE
+                               || fr->frametype == AST_FRAME_VIDEO
+                               || fr->frametype == AST_FRAME_IMAGE) {
                                ao2_cleanup(fr->subclass.format);
                        }
 
@@ -149,8 +153,9 @@ static void __frame_free(struct ast_frame *fr, int cache)
                ast_free((void *) fr->src);
        }
        if (fr->mallocd & AST_MALLOCD_HDR) {
-               if ((fr->frametype == AST_FRAME_VOICE) || (fr->frametype == AST_FRAME_VIDEO) ||
-                       (fr->frametype == AST_FRAME_IMAGE)) {
+               if (fr->frametype == AST_FRAME_VOICE
+                       || fr->frametype == AST_FRAME_VIDEO
+                       || fr->frametype == AST_FRAME_IMAGE) {
                        ao2_cleanup(fr->subclass.format);
                }