3
\$\begingroup\$

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
    }
\$\endgroup\$

1 Answer 1

6
\$\begingroup\$

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.

\$\endgroup\$
2
  • \$\begingroup\$ I don't have control over the external method, that's why I cannot use CancellationToken mechanism. I cannot agree that we should never use Thread.Abort(). We should just avoid using it if that is possible: stackoverflow.com/questions/4359910/… I put all effort to ensure nice cancelation. I try to abort thread only if external method hanged. Then I'm catching ThreadAbortException. The question is, could it be done better ? \$\endgroup\$
    – Kuba
    Sep 30, 2012 at 7:01
  • \$\begingroup\$ @Kuba The best way to ensure nice cancellation of external code is to kill the process :) \$\endgroup\$
    – Carl Walsh
    Apr 7, 2015 at 22:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.