views:

123

answers:

3

I would like to invoke heavy duty method dowork on a separate thread and kill it if its taking longer than 3 seconds. Is there any problem with the following code?

class Class1
{
    /// <summary>
    /// The main entry point for the application.
    /// </summary>
    [STAThread]
    static void Main(string[] args)
    {
        Console.WriteLine("starting new thread");

        Thread t = new Thread(new ThreadStart(dowork));
        t.Start();

        DateTime start = DateTime.Now;
        TimeSpan span = DateTime.Now.Subtract(start);

        bool wait = true;
        while (wait == true)
        {
            if (span.Seconds > 3)
            {
                t.Abort();
                wait = false;
            }
            span = DateTime.Now.Subtract(start);
        }

        Console.WriteLine("ending new thread after seconds = {0}", span.Seconds);
        Console.WriteLine("all done");
        Console.ReadLine();

    }

    static void dowork()
    {

        Console.WriteLine("doing heavy work inside hello");
        Thread.Sleep(7000);
        Console.WriteLine("*** finished**** doing heavy work inside hello");
    }
}
+5  A: 

If it were me, I'd use the Thread.Join() overload that takes a milliseconds parameter. (Or the one that takes a TimeSpan.) It's cleaner and involves fewer lines of code.

t.Start();

if (!t.Join(3000))
    t.Abort();

Or, like Matti said, put the timing logic inside threaded process and have it self-destruct.

Toby
I forgot about using Join with a timeout altogether. This is better than my `Sleep` because it will continue after less than 3 seconds if the operation completes earlier than that.
Matti Virkkunen
A: 

I think you need to learn about Thread.Join(int). It will attempt to join with a thread and after a specified timeout it'll exit, upon which you can call Thread.Abort().

Aren
+10  A: 

This is a very bad programming practice. Do not start up threads that you cannot shut down cleanly. The code which starts the thread and the code which runs the thread should have some well-defined mechanism by which they can communicate. There are many such mechanisms; for example, the worker thread could periodically ask the owner thread "should I keep going?" The worker thread could periodically check a threadsafe field that says whether to halt soon. And so on.

If you are starting up work on a new thread and you do not know what it is doing or how long it is going to take, then you are playing with fire. Particularly if that thread code could be hostile and actively resisting being taken down. There is no guarantee whatsoever that aborting a thread does anything at all; if you are clever then you can write code that makes threads that are resistant to thread aborts.

If you're in that situation, the right thing to do is to start up the code in a new process, not a new thread, and then shut down the process when it takes too long.

Eric Lippert