[asterisk-commits] kharwell: branch certified-11.2 r381705 - in /certified/branches/11.2: ./ apps/

[SVN commits to the Asterisk project]

Author: kharwell
Date: Mon Feb 18 16:41:15 2013
New Revision: 381705

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=381705
Log:
Fixed Confbridge file recording deadlock and appending.

A deadlock occurred after starting/stopping and then restarting a confbridge
recording.  Upon starting a recording a record thread is created that holds a
lock until just before exiting.  Stopping the recording does not stop/exit the
thread or release the lock.  The thread waits until recording begins again.
Starting a stopped recording signals the thread to continue and start recording
again.  However restarting the recording also created another record thread
resulting in a deadlock.  The fix was to make sure the record thread was only
created once.

Also it was noted that filenames for the recordings were being concatenated for
each start/stop.  This was fixed by creating a new file for each conference
session and appending the actual recorded data within the file (e.g. passing
the 'a' option to MixMonitor).

(issue AST-1088)
Reported by: John Bigelow
Review: http://reviewboard.digium.internal/r/374/
........

Merged revisions 381702 from http://svn.asterisk.org/svn/asterisk/branches/11

Modified:
    certified/branches/11.2/   (props changed)
    certified/branches/11.2/apps/app_confbridge.c

Propchange: certified/branches/11.2/
------------------------------------------------------------------------------
--- branch-11-merged (original)
+++ branch-11-merged Mon Feb 18 16:41:15 2013
@@ -1,1 +1,1 @@
-/branches/11:378038,378121,378287,378321,378409-378411,378459,378582,378687,378690,378984,379513,379790,380465,380698,380892,380894,380974,381306,381594,381613
+/branches/11:378038,378121,378287,378321,378409-378411,378459,378582,378687,378690,378984,379513,379790,380465,380698,380892,380894,380974,381306,381594,381613,381702

Modified: certified/branches/11.2/apps/app_confbridge.c
URL: http://svnview.digium.com/svn/asterisk/certified/branches/11.2/apps/app_confbridge.c?view=diff&rev=381705&r1=381704&r2=381705
==============================================================================
--- certified/branches/11.2/apps/app_confbridge.c (original)
+++ certified/branches/11.2/apps/app_confbridge.c Mon Feb 18 16:41:15 2013
@@ -407,6 +407,34 @@
 	return tmp;
 }
 
+static void set_rec_filename(struct conference_bridge *bridge, struct ast_str **filename)
+{
+	char *rec_file = bridge->b_profile.rec_file;
+	time_t now;
+	char *ext;
+
+	if (ast_str_strlen(*filename)) {
+		    return;
+	}
+
+	time(&now);
+
+	ast_str_reset(*filename);
+	if (ast_strlen_zero(rec_file)) {
+		ast_str_set(filename, 0, "confbridge-%s-%u.wav", bridge->name, (unsigned int)now);
+	} else {
+		/* insert time before file extension */
+		ext = strrchr(rec_file, '.');
+		if (ext) {
+			ast_str_set_substr(filename, 0, rec_file, ext - rec_file);
+			ast_str_append(filename, 0, "-%u%s", (unsigned int)now, ext);
+		} else {
+			ast_str_set(filename, 0, "%s-%u", rec_file, (unsigned int)now);
+		}
+	}
+	ast_str_append(filename, 0, ",a");
+}
+
 static void *record_thread(void *obj)
 {
 	struct conference_bridge *conference_bridge = obj;
@@ -425,16 +453,7 @@
 
 	/* XXX If we get an EXIT right here, START will essentially be a no-op */
 	while (conference_bridge->record_state != CONF_RECORD_EXIT) {
-		if (!(ast_strlen_zero(conference_bridge->b_profile.rec_file))) {
-			ast_str_append(&filename, 0, "%s", conference_bridge->b_profile.rec_file);
-		} else {
-			time_t now;
-			time(&now);
-			ast_str_append(&filename, 0, "confbridge-%s-%u.wav",
-				conference_bridge->name,
-				(unsigned int) now);
-		}
-
+		set_rec_filename(conference_bridge, &filename);
 		chan = ast_channel_ref(conference_bridge->record_chan);
 		ast_answer(chan);
 		pbx_exec(chan, mixmonapp, ast_str_buffer(filename));
@@ -565,6 +584,13 @@
 	ao2_ref(conference_bridge, +1); /* give the record thread a ref */
 
 	conf_start_record(conference_bridge);
+
+	/*
+	 * if the thread has already been started, don't start another
+	 */
+	if (conference_bridge->record_thread != AST_PTHREADT_NULL) {
+		return 0;
+	}
 
 	if (ast_pthread_create_background(&conference_bridge->record_thread, NULL, record_thread, conference_bridge)) {
 		ast_log(LOG_WARNING, "Failed to create recording channel for conference %s\n", conference_bridge->name);