<p>Joshua Colp <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/7195">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Corey Farrell: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved; Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">core: Add cache_media_frames debugging option.<br><br>The media frame cache gets in the way of finding use after free errors of<br>media frames. Tools like valgrind and MALLOC_DEBUG don't know when a<br>frame is released because it gets put into the cache instead of being<br>freed.<br><br>* Added the "cache_media_frames" option to asterisk.conf. Disabling the<br>option helps track down media frame mismanagement when using valgrind or<br>MALLOC_DEBUG. The cache gets in the way of determining if the frame is<br>used after free and who freed it. NOTE: This option has no effect when<br>Asterisk is compiled with the LOW_MEMORY compile time option enabled<br>because the cache code does not exist.<br><br>To disable the media frame cache simply disable the cache_media_frames<br>option in asterisk.conf and restart Asterisk.<br><br>Sample asterisk.conf setting:<br>[options]<br>cache_media_frames=no<br><br>ASTERISK-27413<br><br>Change-Id: I0ab2ce0f4547cccf2eb214901835c2d951b78c00<br>---<br>M CHANGES<br>M channels/iax2/parser.c<br>M configs/samples/asterisk.conf.sample<br>M include/asterisk/options.h<br>M main/asterisk.c<br>M main/frame.c<br>6 files changed, 45 insertions(+), 9 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/CHANGES b/CHANGES<br>index 74ebf64..7dd1aac 100644<br>--- a/CHANGES<br>+++ b/CHANGES<br>@@ -12,6 +12,15 @@<br> --- Functionality changes from Asterisk 13.18.0 to Asterisk 13.19.0 ----------<br> ------------------------------------------------------------------------------<br> <br>+Core<br>+------------------<br>+ * Added the "cache_media_frames" option to asterisk.conf. Disabling the option<br>+ helps track down media frame mismanagement when using valgrind or<br>+ MALLOC_DEBUG. The cache gets in the way of determining if the frame is<br>+ used after free and who freed it. NOTE: This option has no effect when<br>+ Asterisk is compiled with the LOW_MEMORY compile time option enabled because<br>+ the cache code does not exist.<br>+<br> res_pjsip<br> ------------------<br> * The "identify_by" on endpoints can now be set to "ip" to restrict an endpoint<br>diff --git a/channels/iax2/parser.c b/channels/iax2/parser.c<br>index 2a3eabf..09c1323 100644<br>--- a/channels/iax2/parser.c<br>+++ b/channels/iax2/parser.c<br>@@ -1298,7 +1298,9 @@<br> ast_atomic_fetchadd_int(&frames, -1);<br> <br> #if !defined(LOW_MEMORY)<br>- if (!fr->cacheable || !(iax_frames = ast_threadstorage_get(&frame_cache, sizeof(*iax_frames)))) {<br>+ if (!fr->cacheable<br>+ || !ast_opt_cache_media_frames<br>+ || !(iax_frames = ast_threadstorage_get(&frame_cache, sizeof(*iax_frames)))) {<br> ast_free(fr);<br> return;<br> }<br>diff --git a/configs/samples/asterisk.conf.sample b/configs/samples/asterisk.conf.sample<br>index 38cee25..e57ee10 100644<br>--- a/configs/samples/asterisk.conf.sample<br>+++ b/configs/samples/asterisk.conf.sample<br>@@ -43,6 +43,15 @@<br> ;minmemfree = 1 ; In MBs, Asterisk stops accepting new calls if<br> ; the amount of free memory falls below this<br> ; watermark.<br>+;cache_media_frames = yes ; Cache media frames for performance<br>+ ; Disable this option to help track down media frame<br>+ ; mismanagement when using valgrind or MALLOC_DEBUG.<br>+ ; The cache gets in the way of determining if the<br>+ ; frame is used after being freed and who freed it.<br>+ ; NOTE: This option has no effect when Asterisk is<br>+ ; compiled with the LOW_MEMORY compile time option<br>+ ; enabled because the cache code does not exist.<br>+ ; Default yes<br> ;cache_record_files = yes ; Cache recorded sound files to another<br> ; directory during recording.<br> ;record_cache_dir = /tmp ; Specify cache directory (used in conjunction<br>diff --git a/include/asterisk/options.h b/include/asterisk/options.h<br>index 950764e..c64de2f 100644<br>--- a/include/asterisk/options.h<br>+++ b/include/asterisk/options.h<br>@@ -76,6 +76,8 @@<br> AST_OPT_FLAG_DONT_WARN = (1 << 18),<br> /*! End CDRs before the 'h' extension */<br> AST_OPT_FLAG_END_CDR_BEFORE_H_EXTEN = (1 << 19),<br>+ /*! Cache media frames for performance */<br>+ AST_OPT_FLAG_CACHE_MEDIA_FRAMES = (1 << 20),<br> /*! Always fork, even if verbose or debug settings are non-zero */<br> AST_OPT_FLAG_ALWAYS_FORK = (1 << 21),<br> /*! Disable log/verbose output to remote consoles */<br>@@ -99,7 +101,7 @@<br> };<br> <br> /*! These are the options that set by default when Asterisk starts */<br>-#define AST_DEFAULT_OPTIONS AST_OPT_FLAG_TRANSCODE_VIA_SLIN<br>+#define AST_DEFAULT_OPTIONS (AST_OPT_FLAG_TRANSCODE_VIA_SLIN | AST_OPT_FLAG_CACHE_MEDIA_FRAMES)<br> <br> #define ast_opt_exec_includes ast_test_flag(&ast_options, AST_OPT_FLAG_EXEC_INCLUDES)<br> #define ast_opt_no_fork ast_test_flag(&ast_options, AST_OPT_FLAG_NO_FORK)<br>@@ -116,6 +118,7 @@<br> #define ast_opt_stdexten_macro ast_test_flag(&ast_options, AST_OPT_FLAG_STDEXTEN_MACRO)<br> #define ast_opt_dump_core ast_test_flag(&ast_options, AST_OPT_FLAG_DUMP_CORE)<br> #define ast_opt_cache_record_files ast_test_flag(&ast_options, AST_OPT_FLAG_CACHE_RECORD_FILES)<br>+#define ast_opt_cache_media_frames ast_test_flag(&ast_options, AST_OPT_FLAG_CACHE_MEDIA_FRAMES)<br> #define ast_opt_timestamp ast_test_flag(&ast_options, AST_OPT_FLAG_TIMESTAMP)<br> #define ast_opt_override_config ast_test_flag(&ast_options, AST_OPT_FLAG_OVERRIDE_CONFIG)<br> #define ast_opt_reconnect ast_test_flag(&ast_options, AST_OPT_FLAG_RECONNECT)<br>diff --git a/main/asterisk.c b/main/asterisk.c<br>index 8cbbe93..0eaf8dd 100644<br>--- a/main/asterisk.c<br>+++ b/main/asterisk.c<br>@@ -676,6 +676,9 @@<br> ast_cli(a->fd, " Transmit silence during rec: %s\n", ast_test_flag(&ast_options, AST_OPT_FLAG_TRANSMIT_SILENCE) ? "Enabled" : "Disabled");<br> ast_cli(a->fd, " Generic PLC: %s\n", ast_test_flag(&ast_options, AST_OPT_FLAG_GENERIC_PLC) ? "Enabled" : "Disabled");<br> ast_cli(a->fd, " Min DTMF duration:: %u\n", option_dtmfminduration);<br>+#if !defined(LOW_MEMORY)<br>+ ast_cli(a->fd, " Cache media frames: %s\n", ast_opt_cache_media_frames ? "Enabled" : "Disabled");<br>+#endif<br> <br> if (ast_option_rtpptdynamic == AST_RTP_PT_LAST_REASSIGN) {<br> ast_cli(a->fd, " RTP dynamic payload types: %u,%u-%u\n",<br>@@ -3827,6 +3830,11 @@<br> /* Cache recorded sound files to another directory during recording */<br> } else if (!strcasecmp(v->name, "cache_record_files")) {<br> ast_set2_flag(&ast_options, ast_true(v->value), AST_OPT_FLAG_CACHE_RECORD_FILES);<br>+#if !defined(LOW_MEMORY)<br>+ /* Cache media frames for performance */<br>+ } else if (!strcasecmp(v->name, "cache_media_frames")) {<br>+ ast_set2_flag(&ast_options, ast_true(v->value), AST_OPT_FLAG_CACHE_MEDIA_FRAMES);<br>+#endif<br> /* Specify cache directory */<br> } else if (!strcasecmp(v->name, "record_cache_dir")) {<br> ast_copy_string(record_cache_dir, v->value, AST_CACHE_DIR_LEN);<br>diff --git a/main/frame.c b/main/frame.c<br>index 4261b04..db2b967 100644<br>--- a/main/frame.c<br>+++ b/main/frame.c<br>@@ -122,14 +122,18 @@<br> return;<br> <br> #if !defined(LOW_MEMORY)<br>- if (cache && fr->mallocd == AST_MALLOCD_HDR) {<br>+ if (fr->mallocd == AST_MALLOCD_HDR<br>+ && cache<br>+ && ast_opt_cache_media_frames) {<br> /* Cool, only the header is malloc'd, let's just cache those for now<br> * to keep things simple... */<br> struct ast_frame_cache *frames;<br>- if ((frames = ast_threadstorage_get(&frame_cache, sizeof(*frames))) &&<br>- (frames->size < FRAME_CACHE_MAX_SIZE)) {<br>- if ((fr->frametype == AST_FRAME_VOICE) || (fr->frametype == AST_FRAME_VIDEO) ||<br>- (fr->frametype == AST_FRAME_IMAGE)) {<br>+<br>+ frames = ast_threadstorage_get(&frame_cache, sizeof(*frames));<br>+ if (frames && frames->size < FRAME_CACHE_MAX_SIZE) {<br>+ if (fr->frametype == AST_FRAME_VOICE<br>+ || fr->frametype == AST_FRAME_VIDEO<br>+ || fr->frametype == AST_FRAME_IMAGE) {<br> ao2_cleanup(fr->subclass.format);<br> }<br> <br>@@ -149,8 +153,9 @@<br> ast_free((void *) fr->src);<br> }<br> if (fr->mallocd & AST_MALLOCD_HDR) {<br>- if ((fr->frametype == AST_FRAME_VOICE) || (fr->frametype == AST_FRAME_VIDEO) ||<br>- (fr->frametype == AST_FRAME_IMAGE)) {<br>+ if (fr->frametype == AST_FRAME_VOICE<br>+ || fr->frametype == AST_FRAME_VIDEO<br>+ || fr->frametype == AST_FRAME_IMAGE) {<br> ao2_cleanup(fr->subclass.format);<br> }<br> <br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/7195">change 7195</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/7195"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I0ab2ce0f4547cccf2eb214901835c2d951b78c00 </div>
<div style="display:none"> Gerrit-Change-Number: 7195 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>