Sequential async task queue does not wait until async task ends

Question

I am trying to sequentially queue async tasks so I made a class for this:

 public class TaskSequentialQueue : IDisposable, ITaskSequentialQueue
    {
        public delegate void OnExeptionDelegate(Exception ex);

        private readonly Queue<Task> m_queue = new Queue<Task>();
        private readonly Object m_lock = new Object();
        private readonly CancellationTokenSource m_CancelToken = new CancellationTokenSource();
        private readonly OnExeptionDelegate m_onExeptionDelegate = null;
        private Task m_currentTask = null;
        private bool m_isDisposed = false;      

        public TaskSequentialQueue(OnExeptionDelegate expDelegate = null)
        {
            m_onExeptionDelegate = expDelegate;
        }

        public void Dispose()
        {
            Dispose(true);
            GC.SuppressFinalize(this);
        }

        protected virtual void Dispose(bool isDisposing)
        {
            if (m_isDisposed)
            {
                return;
            }

            if (isDisposing)
            {
                lock (m_lock)
                {
                    m_isDisposed = true;
                    m_queue.Clear();
                }

                m_CancelToken.Cancel();
                m_CancelToken.Dispose();
            }
        }

        public void EnqueueTask( Task task)
        {
            lock (m_lock)
            {
                if (m_isDisposed)
                    throw new ObjectDisposedException("TaskSequentialQueue");

                m_queue.Enqueue(task);
            }

            StartNextTask();
        }

        public void EnqueueTask( Func<Task> task)
        {
            EnqueueTask(new Task<Task>(task));
        }

        public Task EnqueueTaskAndWait( Task task)
        {
            TaskCompletionSource<int> taskSource = new TaskCompletionSource<int>();
            lock (m_lock)
            {
                if (m_isDisposed)
                    throw new ObjectDisposedException("TaskSequentialQueue");

                Func<Task> toDo = async () =>
                {                                        
                    var waitabletask = task.ContinueWith( antecedent => 
                                                            { 
                                                                taskSource.SetResult(0); 
                                                                if (antecedent.Exception != null)
                                                                    throw antecedent.Exception; 
                                                            });
                    task.Start();
                    await waitabletask;
                };
                this.EnqueueTask(toDo);
            }

            StartNextTask();

            return taskSource.Task;  //TODO! propagate the exception correctly ?
        }

        private void StartNextTask()
        {
            Task theTask = null;
            lock(m_lock)
            {
                if (m_currentTask == null && m_queue.Count > 0 && !m_isDisposed)
                {
                    m_currentTask = m_queue.Dequeue();
                    theTask = m_currentTask;
                }
            }      
            if (theTask != null)
            {
                theTask.Start();  
                theTask.ContinueWith( (antecedent) =>
                    {
                        Exception theEx = antecedent.Exception;
                        if (theEx == null && antecedent is Task<Task>)
                            theEx = (antecedent as Task<Task>)?.Result.Exception;

                        if (m_onExeptionDelegate != null && theEx != null)
                        {
                            try { m_onExeptionDelegate(theEx); } catch(Exception) {}
                        }

                        lock(m_lock)
                        {
                            m_currentTask = null;
                        }

                        Task.Run( () =>  StartNextTask() );
                }
            }
        }
    }

I use it like this:

     Func<Task> action = async () =>
                {
                   Log("Entered");
                   await Task.Delay(5000);
                   Log("Exited");
                }

m_taskSequentialQueue.EnqueueTask( action );
m_taskSequentialQueue.EnqueueTask( action );

I expect my log to read:

Entered
Exited
Entered
Exited

Instead I get:

Entered
Entered
Exited
Exited

I'm not sure what I'm doing wrong.

Thanks

Doing a `Queue` seems very odd to me, you should rarely have "cold tasks" that are not running, have you considered switching to a `Queue>` and dropping the `public void EnqueueTask( Task task)` overload? — Scott Chamberlain, Sep 01 '16 at 15:17
From what I see you lock on the queueing and dequeueing but not on the actual execution which returns immediatly because you do not await or retrieve the results for the task to run synchronously. I have to also agree with Scott, this seems like an odd construct that does not add any real benefit. What is it you are trying to achieve/build? A synchronous queue that executes Tasks/Functions? — Igor, Sep 01 '16 at 15:24
Also there are synchronized versions available for Queue so you do not have to lock explicitly when queuing and dequeuing. `System.Collections.Concurrent.ConcurrentQueue<>` — Igor, Sep 01 '16 at 15:27

score 4 · Accepted Answer · answered Sep 01 '16 at 15:29

When you do theTask.ContinueWith( in StartNextTask the thing you are continuing on is the starting of the inner task not the completion of the inner task. Once the inner task hits the first await the theTask task will be considered complete because the function returned.

As a band-aid you could do

if (theTask != null)
{
    theTask.Start();  
    if(theTask is Task<Task>)
    {
       theTask = ((Task<Task>)theTask).Unwrap();
    }
    theTask.ContinueWith(...

However I think your whole method of using "Cold Tasks" is flawed. You should just work with a Queue<Func<Task>> instead of a Queue<Task> and it will allow your code to be much simpler.

score 0 · Answer 2 · answered Sep 01 '16 at 15:31

        Func<Task> action = async () =>
        {
            lock (lock_x)
            {
                Console.WriteLine("Entered");
                Thread.Sleep(5000);
                Console.WriteLine("Exited");
            }
        };

should work since your queue implementation is submitting them in order.

Sequential async task queue does not wait until async task ends

2 Answers2

Linked