[Asterisk-code-review] core: Add cache media frames debugging option. (asterisk[15])
Jenkins2
asteriskteam at digium.com
Tue Nov 14 07:13:55 CST 2017
Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/7196 )
Change subject: core: Add cache_media_frames debugging option.
......................................................................
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
---
M CHANGES
M channels/iax2/parser.c
M configs/samples/asterisk.conf.sample
M include/asterisk/options.h
M main/asterisk.c
M main/frame.c
6 files changed, 45 insertions(+), 9 deletions(-)
Approvals:
Corey Farrell: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved
Jenkins2: Approved for Submit
diff --git a/CHANGES b/CHANGES
index 30d3493..8a7fb31 100644
--- a/CHANGES
+++ b/CHANGES
@@ -12,6 +12,15 @@
--- Functionality changes from Asterisk 15.1.0 to Asterisk 15.2.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_rtp_asterisk
------------------
* The X.509 certificate used for DTLS negotation can now be automatically
diff --git a/channels/iax2/parser.c b/channels/iax2/parser.c
index ec9d346..6eda982 100644
--- a/channels/iax2/parser.c
+++ b/channels/iax2/parser.c
@@ -1296,7 +1296,9 @@
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 30934e4..8379b6e 100644
--- a/configs/samples/asterisk.conf.sample
+++ b/configs/samples/asterisk.conf.sample
@@ -44,6 +44,15 @@
;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 0a20f10..878748d 100644
--- a/include/asterisk/options.h
+++ b/include/asterisk/options.h
@@ -66,6 +66,8 @@
AST_OPT_FLAG_CACHE_RECORD_FILES = (1 << 13),
/*! Display timestamp in CLI verbose output */
AST_OPT_FLAG_TIMESTAMP = (1 << 14),
+ /*! Cache media frames for performance */
+ AST_OPT_FLAG_CACHE_MEDIA_FRAMES = (1 << 15),
/*! Reconnect */
AST_OPT_FLAG_RECONNECT = (1 << 16),
/*! Transmit Silence during Record() and DTMF Generation */
@@ -99,7 +101,7 @@
};
/*! 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 @@
#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_reconnect ast_test_flag(&ast_options, AST_OPT_FLAG_RECONNECT)
#define ast_opt_transmit_silence ast_test_flag(&ast_options, AST_OPT_FLAG_TRANSMIT_SILENCE)
diff --git a/main/asterisk.c b/main/asterisk.c
index 40986a4..20ed947 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -607,6 +607,9 @@
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
ast_cli(a->fd, " RTP use dynamic payloads: %u\n", ast_option_rtpusedynamic);
if (ast_option_rtpptdynamic == AST_RTP_PT_LAST_REASSIGN) {
@@ -3714,6 +3717,11 @@
/* 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 c24cc8f..690d48e 100644
--- a/main/frame.c
+++ b/main/frame.c
@@ -120,14 +120,18 @@
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);
}
@@ -147,8 +151,9 @@
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);
}
--
To view, visit https://gerrit.asterisk.org/7196
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: merged
Gerrit-Change-Id: I0ab2ce0f4547cccf2eb214901835c2d951b78c00
Gerrit-Change-Number: 7196
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171114/25ccd54a/attachment.html>
More information about the asterisk-code-review
mailing list