Thread termination

[RedLars]

Hi,

In the below example, what is the preferred approach to ending the
thread execution from the Dispose method? Using Thread.Abort would be
one appraoch. Changing while(true) to while(SomeVariable) and then
control SomeVariable from dispose(..) would be another way. Maybe
using some sort of synchronize mechanism also would work? What do you
recommend?

Thread pollDeviceThread = new Thread(new ThreadStart(PollDevice));

private void PollDevice()
{
while(true)
{
// Do work
Thread.Sleep(POLL_DEVICE_UPDATE_TIME);
}
}

protected virtual void Dispose(bool disposingManagedResource)
{

}

 

[Peter Morris]

It depends on how long your poll time is. If it is a short time then I
would have a

while (!Terminated)
{
.........
}

If it is too long I would create a ManualResetEvent

public class MyWorker
{
private volatile bool Terminated;
private readonly ManualResetEvent TerminateResetEvent;

public MyWorker()
{
..Create the TerminateResetEvent;
}

public void Execute()
{
while (!Terminated)
{
..Do stuff
Monitor.Wait(TerminateResetEvent, poll time);
}
}

public void Terminate()
{
if (!Terminated)
{
Terminated = true;
TerminateResetEvent.Set();
}
}
}


As usual the code is just an idea, probably not compilable etc.

 

[RedLars]

Instead of Thread.Sleep(), you can use something like WaitHandle.WaitOne()  
with a timeout equal to your "POLL_DEVICE_UPDATE_TIME".  Then in the  
Dispose() method, set the WaitHandle (for example, use an AutoResetEvent)..

You can combine this with a volatile flag as well, or you can just rely on  
the return value from WaitOne() (which will tell you whether it timed out 
or you were signaled by the WaitHandle) to know when to break out of the  
loop.  Using a flag would allow you to ensure that even if the signaling  
of the WaitHandle loses the race with the timeout that you exit  
immediately, but it's probably the case that polling the device one extra 
time before the thread exits is not a problem.

Note here that I'm assuming terminating the thread from the Dispose()  
method is the right thing to do, as is polling your device rather than  
using some other form of i/o.  For example, you've got some object that 
wraps your device and incorporates internally a thread to support the  
public interface offered by the class, a thread that needs to be shut down  
when the class is disposed (though maybe even in that case, that would be 
a rare example of a case where having a finalizer but no Dispose() method 
might make sense, providing a different, clearer public interface for  
shutting down the polling).

Pete

Thanks for the reply Pete.

So basically WaitHandle contains both the timer and the termination
mechanism rolled into one.

How would the dispose logic work for this scenario? How can I both
signal the thread to terminate and dispose of the AutoResetEvent
object? Tried to illustrate my point below, the autoResetEvent.Dispose
() method shouldnt really be called before the poll-thread has
received the automResetEvent signal and terminated.

protected virtual void Dispose(bool disposeManagedResources)
{
if (!(disposed))
{
if (disposeManagedResources)
{
/* Signal thread to terminate */
autoResetEvent.set();

/* Release resource - this cannot work??? */
autoResetEvent.Dispose();
}
}
}

 

[RedLars]

It depends on how long your poll time is.  If it is a short time then I
would have a

while (!Terminated)
{
    .........

}

If it is too long I would create a ManualResetEvent

public class MyWorker
{
    private volatile bool Terminated;
    private readonly ManualResetEvent TerminateResetEvent;

    public MyWorker()
    {
        ..Create the TerminateResetEvent;
    }

    public void Execute()
    {
        while (!Terminated)
        {
            ..Do stuff
            Monitor.Wait(TerminateResetEvent, poll time);
        }
    }

    public void Terminate()
    {
        if (!Terminated)
        {
            Terminated = true;
            TerminateResetEvent.Set();
        }
    }

}

As usual the code is just an idea, probably not compilable etc.

Thanks for the reply.

Timeout in this particular instance is10 seconds.

 

[Peter Morris]

Timeout in this particular instance is10 seconds.

It's not how long it is that really matters, it's how long you think is
acceptible to wait

 

[RedLars]

It's not how long it is that really matters, it's how long you think is
acceptible to wait

Thanks for the example code.

 

[RedLars]

Close enough.  


Well, as I hinted at before, doing thread signaling in the Dispose()  
method is a potentially "iffy" operation.  This is one of the reasons: to  
correctly manage it may require other deviations from a proper Dispose()  
pattern.

That said, one possible approach would be for you to let the thread  
dispose the WaitHandle instance.  Another possible approach would be to 
have the Dispose() method wait on the thread to exit (Thread.Join()), and 
only dispose the WaitHandle after the thread has exited.

Personally, I'd prefer the former...having a Dispose() method able to  
block in any way just seems wrong to me.

It's not that it can't work.  It's just that whatever you wind up with, 
it's not going to be as clean as a normal Dispose() implementation,  
because you're already doing something unusual in your Dispose() method.

Pete

Thanks for your input.

Would it be better to split the task into two. Use a stop() method
that does the signaling to the thread and the dispose does, well, the
disposing of AutoResetEvent and other objects? The problem with that
approach is that Dispose sort of requires the stop method to be
executed to kill the tread before Dispose is executed. Thoughts?

 

[RedLars]

One possibility is to throw an InvalidOperationException if Dispose() is  
called while the thread is still running.

However, I think that's kind of ugly.  I think I would have a Stop()  
method that does the signaling and as I mentioned, lets the thread dispose  
the AutoResetEvent, and not have a Dispose() method at all.  Then the  
finalizer can just stop the thread, secure in the knowledge that the  
thread will dispose on the AutoResetEvent on its way out.

Pete

Thanks again for your reply.

Not sure I follow.

Do you mean the spawned thread should clean-up the AutoResetEvent
object and not implement IDisposable at all? Would not that break with
the rules for when to use IDisposable?