Cancelable thread worker

Question

My goal was to create class, that can run external method in non blocking manner. Second requirement was to be able to abort method run if it is needed (i.e. running user script). I decided to do that with Thread instead of Task, because only Thread can be truly aborted. Because Thread have big creation overhead, it is initialized and started in advance and resumed if work is needed.

What do you think about my implementation ?

Example usage:

//generic type param is used as external method argument (instead of object)
AbortableStepWorker<int> worker = new AbortableStepWorker<int>(someMethodWithIntArgument);
worker.Start();
worker.DoWork(someArgument);
while(!worker.WaitForWorkDone(someTimeout))
    if(cancelWasRequested)
    {
        worker.Stop(lastChanceTimeout);
        return false;
    }

//if we've managed to get here, then work was done
//run result:
var result = !worker.Stopped;

and class implementation:

public class AbortableStepWorker<T> : IDisposable
    {
        public AbortableStepWorker(Func<T, bool> externalWork)
        {
            if (externalWork == null) throw new ArgumentNullException("workMethod");
            this.externalWork = externalWork;

            ThreadInstance = new Thread(Worker) { Priority = ThreadPriority.BelowNormal };
        }

        private readonly Func<T, bool> externalWork;
        private T externalWorkParam;
        private void Worker()
        {
            try
            {
                while (true)
                {
                    resumeEvent.WaitOne(Timeout.Infinite);

                    if (stopRequested)
                        return;

                    if (!externalWork(externalWorkParam))
                        return;

                    workDoneEvent.Set();
                }
            }
            catch (ThreadAbortException) { }
            finally { workDoneEvent.Set(); }
        }

        private bool stopRequested = false;
        private AutoResetEvent resumeEvent = new AutoResetEvent(false);
        private ManualResetEvent workDoneEvent = new ManualResetEvent(false);

        public Thread ThreadInstance { get; private set; }

        public void Start()
        {
            ThreadInstance.Start();
        }

        /// <summary>
        /// Allows inner thread to do externalWork call.
        /// </summary>
        public void DoWork(T param)
        {
            externalWorkParam = param;
            workDoneEvent.Reset();
            resumeEvent.Set();
        }

        /// <summary>
        /// Waits for DoWork to finish.
        /// </summary>
        /// <param name="millisecondsTimeout">Timeout.</param>
        /// <returns>True if work is not still running.</returns>
        public bool WaitForWorkDone(int millisecondsTimeout)
        {
            return workDoneEvent.WaitOne(millisecondsTimeout);
        }

        /// <summary>
        /// Waits specified time for worker method to end. If time elapsed, aborts thread.
        /// </summary>
        /// <param name="millisecondsTimeout">Timeout before thread abort.</param>
        public void Stop(int millisecondsTimeout = 0)
        {
            if (Stopped)
                return;

            bool wasWorking = !workDoneEvent.WaitOne(0);
            stopRequested = true;
            resumeEvent.Set();

            if (!ThreadInstance.Join(wasWorking ? millisecondsTimeout : Timeout.Infinite))
                ThreadInstance.Abort();
        }

        public bool Stopped
        {
            get
            {
                return ThreadInstance.Join(0);
            }
        }

        #region IDisposable
        public void Dispose()
        {
            if (ThreadInstance.ThreadState != ThreadState.Unstarted)
            {
                Stop();
                ThreadInstance.Join();
            }
            resumeEvent.Dispose();
            workDoneEvent.Dispose();
        }
        #endregion //IDisposable
    }

Answer 1

You should never use Thread.Abort(). If you don't have control over the code that's executing in the thread, then you don't know what will that abort do. It's quite possible it will leave some object in an inconsistent state, or something like that. If you do have control over the code, you should instead use cooperative cancellation using CancellationToken.