views:

306

answers:

3

I did some research and looking around and it seems the way to do this is using an AutoResetEvent. I quickly put this together and it seems to work and seems to be thread-safe. Can I have some feedback?

class Program
{
    private Thread workerThread;
    private AutoResetEvent aResetEvent;
    private bool _continueProcessing;
    private bool active;
    private Object locker = new Object();

    public Program()
    {
        workerThread = new Thread(DoSomeProcessing);
        workerThread.IsBackground = true;
        aResetEvent = new AutoResetEvent(false);
    }

    public bool ContinueProcessing
    {
        get
        {
            lock (locker)
            {
                return _continueProcessing;
            }
        }
        set 
        {
            if (value)
            {
                aResetEvent.Set();
            }
            else
            {

                aResetEvent.Reset();
            }
            lock (locker)
            {
                _continueProcessing = value;
            }
        }
    }

    public void DoSomeProcessing()
    {
        int i = 0;
        try
        {
            while (active)
            {
                aResetEvent.WaitOne();
                // do some work and sleep
                lock (locker)
                {
                    if (ContinueProcessing)
                    {
                        aResetEvent.Set();
                    }
                }
            }
        }
        catch(ThreadInterruptedException tie)
        {
            Console.WriteLine("Shutting down.");
        }
        // any shutdown processing            
    }

    public void StopProcessing()
    {
        workerThread.Interrupt();
        workerThread.Join();
    }

    public void PauseProcessing()
    {
        ContinueProcessing = false;
    }

    public void Continue()
    {
        ContinueProcessing = true;
    }

    public void StartProcessing()
    {
        ContinueProcessing = true;
        active = true;
    }
}

EDIT: Hi Again. I have used the feedback and I am much more satisfied with my implementation. Just one little thing that I would like to add, when I pause I would like to wait to make sure that the thread has paused and is no longer doing work. Is that possible? Maybe I should just replace the pause and resume with only start and stop and then on the stop do a thred.join(). Comments?

A: 

looks like overly complex

and

 public void StopProcessing() 
    { 
        workerThread.Interrupt(); 
        workerThread.Join(); 
    } 

can be removed if you just let the thread exit the method

MaLio
A: 

If you changed to using a ManualResetEvent you could remove the _continueProcessing variable. In the set just call Set or Reset on the event. In the getter you can Return aResetEvent.WaitOne(0). You could then remove the piece of code at the end of DoSomeProcessing that Sets the event if processing should contine. Also, becuase ManualResetEvent is itself thread safe, you could completely remove your locking.

Regarding exiting your DoSomeProcessing method. Probably the best thing to do is to use a flag that you set to tell the loop to exit, test the flag from withing a lock at the start of the loop (yes, you now have to put some locking back) and when you want to abort, you set the flag, then set the event.

Alternativly, you could use another event to signal that the loop should exit and change your wait to use WaitHandle.WaitAny().

pipTheGeek
+2  A: 

Once exit is called, the ManualResetEvent will get disposed and exceptions may be thrown on the instance methods when invoked. --> this may not be desirable in some instances

 class Program {
        static void Main(string[] args) {

            //NOTE: if worker goes out of scope it will be collected -> ex: promote to field in real use
            Worker worker = new Worker();
            System.Threading.Thread workerThread = new System.Threading.Thread(new System.Threading.ThreadStart(worker.DoWork));
            workerThread.IsBackground = true;
            workerThread.Start();

            // test 
            worker.Resume();
            System.Threading.Thread.Sleep(2000);

            worker.Pause();
            System.Threading.Thread.Sleep(2000);

            worker.Resume();
            System.Threading.Thread.Sleep(2000);

            worker.Exit();
            System.Threading.Thread.Sleep(5000);        
        }
    }

    public class Worker {

        private readonly System.Threading.ManualResetEvent _Gate;
        private bool _IsActive;

        public Worker() {

            _Gate = new System.Threading.ManualResetEvent(false);
            _IsActive = true;
        }

        public void DoWork() {

            while (IsActive) {
                _Gate.WaitOne();
                // do work

                // can yield the thread 
                System.Threading.Thread.Sleep(1);
            }

            // dispose
            _Gate.Close();
        }

        private bool IsActive {
            get {
                lock (_Gate) {
                    return _IsActive;
                }
            }
        }

        public void Pause() {
            _Gate.Reset();
        }

        public void Resume() {
            _Gate.Set();
        }

        public void Exit() {
            lock (_Gate) {
                _IsActive = false;
            }
        }
    }
MaLio