Frame deferral: Revert API refactoring.

There are several issues with deferring frames that are caused by the
refactoring.

1) The code deferring frames mishandles adding a deferred frame to the
deferred queue.  As a result the deferred queue can only be one frame
long.

2) Deferrable frames can come directly from the channel driver as well as
the read queue.  These frames need to be added to the deferred queue.

3) Whoever is deferring frames is really only doing the __ast_read() to
collect deferred frames and doesn't care about the returned frames except
to detect a hangup event.  When frame deferral is completed we must make
the normal frame processing see the hangup as a frame anyway.  As such,
there is no need to have varying hangup frame deferral methods.  We also
need to be aware of the AST_SOFTHANGUP_ASYNCGOTO hangup that isn't real.
That fake hangup is to cause the PBX thread to break out of loops to go
execute a new dialplan location.

4) To properly deal with deferrable frames from the channel driver as
pointed out by (2) above, means that it is possible to process a dialplan
interception routine while frames are deferred because of the
AST_CONTROL_READ_ACTION control frame.  Deferring frames is not
implemented as a re-entrant operation so you could have the unsupported
case of two sections of code thinking they have control of the media
stream.

A worse problem is because of the bad implementation of the AMI PlayDTMF
action.  It can cause two threads to be deferring frames on the same
channel at the same time.  (ASTERISK_25940)

* Rather than fix all these problems simply revert the API refactoring as
there is going to be only autoservice and safe_sleep deferring frames
anyway.

ASTERISK-26343

ASTERISK-26716 #close

Change-Id: I45069c779aa3a35b6c863f65245a6df2c7865496

1 files changed, 27 insertions, 79 deletions

diff --git a/main/channel.c b/main/channel.c
index 6e88a2906..7c8d3a989 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -1062,25 +1062,6 @@ struct ast_channel *__ast_dummy_channel_alloc(const char *file, int line, const
 	return tmp;
 }
 
-void ast_channel_start_defer_frames(struct ast_channel *chan, int defer_hangups)
-{
-	ast_set_flag(ast_channel_flags(chan), AST_FLAG_DEFER_FRAMES);
-	ast_set2_flag(ast_channel_flags(chan), defer_hangups, AST_FLAG_DEFER_HANGUP_FRAMES);
-}
-
-void ast_channel_stop_defer_frames(struct ast_channel *chan)
-{
-	struct ast_frame *f;
-
-	ast_clear_flag(ast_channel_flags(chan), AST_FLAG_DEFER_FRAMES);
-
-	/* Move the deferred frames onto the channel read queue, ahead of other queued frames */
-	while ((f = AST_LIST_REMOVE_HEAD(ast_channel_deferred_readq(chan), frame_list))) {
-		ast_queue_frame_head(chan, f);
-		ast_frfree(f);
-	}
-}
-
 static int __ast_queue_frame(struct ast_channel *chan, struct ast_frame *fin, int head, struct ast_frame *after)
 {
 	struct ast_frame *f;
@@ -1545,18 +1526,19 @@ int ast_safe_sleep_conditional(struct ast_channel *chan, int timeout_ms, int (*c
 	int res = 0;
 	struct timeval start;
 	int ms;
+	AST_LIST_HEAD_NOLOCK(, ast_frame) deferred_frames;
+
+	AST_LIST_HEAD_INIT_NOLOCK(&deferred_frames);
 
 	/* If no other generator is present, start silencegen while waiting */
 	if (ast_opt_transmit_silence && !ast_channel_generatordata(chan)) {
 		silgen = ast_channel_start_silence_generator(chan);
 	}
 
-	ast_channel_lock(chan);
-	ast_channel_start_defer_frames(chan, 0);
-	ast_channel_unlock(chan);
-
 	start = ast_tvnow();
 	while ((ms = ast_remaining_ms(start, timeout_ms))) {
+		struct ast_frame *dup_f = NULL;
+
 		if (cond && ((*cond)(data) == 0)) {
 			break;
 		}
@@ -1571,7 +1553,18 @@ int ast_safe_sleep_conditional(struct ast_channel *chan, int timeout_ms, int (*c
 				res = -1;
 				break;
 			}
-			ast_frfree(f);
+
+			if (!ast_is_deferrable_frame(f)) {
+				ast_frfree(f);
+				continue;
+			}
+
+			if ((dup_f = ast_frisolate(f))) {
+				if (dup_f != f) {
+					ast_frfree(f);
+				}
+				AST_LIST_INSERT_HEAD(&deferred_frames, dup_f, frame_list);
+			}
 		}
 	}
 
@@ -1580,8 +1573,17 @@ int ast_safe_sleep_conditional(struct ast_channel *chan, int timeout_ms, int (*c
 		ast_channel_stop_silence_generator(chan, silgen);
 	}
 
+	/* We need to free all the deferred frames, but we only need to
+	 * queue the deferred frames if there was no error and no
+	 * hangup was received
+	 */
 	ast_channel_lock(chan);
-	ast_channel_stop_defer_frames(chan);
+	while ((f = AST_LIST_REMOVE_HEAD(&deferred_frames, frame_list))) {
+		if (!res) {
+			ast_queue_frame_head(chan, f);
+		}
+		ast_frfree(f);
+	}
 	ast_channel_unlock(chan);
 
 	return res;
@@ -3883,36 +3885,6 @@ static struct ast_frame *__ast_read(struct ast_channel *chan, int dropaudio)
 	if (!AST_LIST_EMPTY(ast_channel_readq(chan))) {
 		int skip_dtmf = should_skip_dtmf(chan);
 
-		if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_DEFER_FRAMES)) {
-			AST_LIST_TRAVERSE_SAFE_BEGIN(ast_channel_readq(chan), f, frame_list) {
-				if (ast_is_deferrable_frame(f)) {
-					if(f->frametype == AST_FRAME_CONTROL && 
-						(f->subclass.integer == AST_CONTROL_HANGUP ||
-						 f->subclass.integer == AST_CONTROL_END_OF_Q)) {
-						/* Hangup is a special case. We want to defer the frame, but we also do not
-						 * want to remove it from the frame queue. So rather than just moving the frame
-						 * over, we duplicate it and move the copy to the deferred readq.
-						 *
-						 * The reason for this? This way, whoever calls ast_read() will get a NULL return
-						 * immediately and can tell the channel has hung up and do what it needs to. Also,
-						 * when frame deferral finishes, then whoever calls ast_read() next will also get
-						 * the hangup.
-						 */
-						if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_DEFER_HANGUP_FRAMES)) {
-							struct ast_frame *dup;
-
-							dup = ast_frdup(f);
-							AST_LIST_INSERT_HEAD(ast_channel_deferred_readq(chan), dup, frame_list);
-						}
-					} else {
-						AST_LIST_INSERT_HEAD(ast_channel_deferred_readq(chan), f, frame_list);
-						AST_LIST_REMOVE_CURRENT(frame_list);
-					}
-				}
-			}
-			AST_LIST_TRAVERSE_SAFE_END;
-		}
-
 		AST_LIST_TRAVERSE_SAFE_BEGIN(ast_channel_readq(chan), f, frame_list) {
 			/* We have to be picky about which frame we pull off of the readq because
 			 * there are cases where we want to leave DTMF frames on the queue until
@@ -10361,15 +10333,9 @@ int ast_channel_connected_line_macro(struct ast_channel *autoservice_chan, struc
 
 		ast_party_connected_line_copy(ast_channel_connected(macro_chan), connected);
 	}
-	ast_channel_start_defer_frames(macro_chan, 0);
 	ast_channel_unlock(macro_chan);
 
 	retval = ast_app_run_macro(autoservice_chan, macro_chan, macro, macro_args);
-
-	ast_channel_lock(macro_chan);
-	ast_channel_stop_defer_frames(macro_chan);
-	ast_channel_unlock(macro_chan);
-
 	if (!retval) {
 		struct ast_party_connected_line saved_connected;
 
@@ -10417,15 +10383,9 @@ int ast_channel_redirecting_macro(struct ast_channel *autoservice_chan, struct a
 
 		ast_party_redirecting_copy(ast_channel_redirecting(macro_chan), redirecting);
 	}
-	ast_channel_start_defer_frames(macro_chan, 0);
 	ast_channel_unlock(macro_chan);
 
 	retval = ast_app_run_macro(autoservice_chan, macro_chan, macro, macro_args);
-
-	ast_channel_lock(macro_chan);
-	ast_channel_stop_defer_frames(macro_chan);
-	ast_channel_unlock(macro_chan);
-
 	if (!retval) {
 		struct ast_party_redirecting saved_redirecting;
 
@@ -10466,15 +10426,9 @@ int ast_channel_connected_line_sub(struct ast_channel *autoservice_chan, struct
 
 		ast_party_connected_line_copy(ast_channel_connected(sub_chan), connected);
 	}
-	ast_channel_start_defer_frames(sub_chan, 0);
 	ast_channel_unlock(sub_chan);
 
 	retval = ast_app_run_sub(autoservice_chan, sub_chan, sub, sub_args, 0);
-
-	ast_channel_lock(sub_chan);
-	ast_channel_stop_defer_frames(sub_chan);
-	ast_channel_unlock(sub_chan);
-
 	if (!retval) {
 		struct ast_party_connected_line saved_connected;
 
@@ -10515,15 +10469,9 @@ int ast_channel_redirecting_sub(struct ast_channel *autoservice_chan, struct ast
 
 		ast_party_redirecting_copy(ast_channel_redirecting(sub_chan), redirecting);
 	}
-	ast_channel_start_defer_frames(sub_chan, 0);
 	ast_channel_unlock(sub_chan);
 
 	retval = ast_app_run_sub(autoservice_chan, sub_chan, sub, sub_args, 0);
-
-	ast_channel_lock(sub_chan);
-	ast_channel_stop_defer_frames(sub_chan);
-	ast_channel_unlock(sub_chan);
-
 	if (!retval) {
 		struct ast_party_redirecting saved_redirecting;