158

I am looking for good ideas for implementing a generic way to have a single line (or anonymous delegate) of code execute with a timeout.

TemperamentalClass tc = new TemperamentalClass();
tc.DoSomething();  // normally runs in 30 sec.  Want to error at 1 min

I'm looking for a solution that can elegantly be implemented in many places where my code interacts with temperamental code (that I can't change).

In addition, I would like to have the offending "timed out" code stopped from executing further if possible.

TheSoftwareJedi
  • 34,421
  • 21
  • 109
  • 151
chilltemp
  • 8,854
  • 8
  • 41
  • 46
  • 46
    Just a reminder to anyone looking at the answers below: Many of them use Thread.Abort which can be very bad. Please read the various comments about this before implementing Abort in your code. It can be appropriate on occasions, but those are rare. If you don't understand exactly what Abort does or don't need it, please implement one of the solutions below that doesn't use it. They are the solutions that don't have as many votes because they didn't fit the needs of my question. – chilltemp Apr 15 '10 at 21:59
  • 7
    For details on the dangers of thread.Abort, read this article from Eric Lippert: http://blogs.msdn.com/b/ericlippert/archive/2010/02/22/should-i-specify-a-timeout.aspx – JohnW Nov 14 '11 at 13:49

7 Answers7

96

The really tricky part here was killing the long running task through passing the executor thread from the Action back to a place where it could be aborted. I accomplished this with the use of a wrapped delegate that passes out the thread to kill into a local variable in the method that created the lambda.

I submit this example, for your enjoyment. The method you are really interested in is CallWithTimeout. This will cancel the long running thread by aborting it, and swallowing the ThreadAbortException:

Usage:

class Program
{

    static void Main(string[] args)
    {
        //try the five second method with a 6 second timeout
        CallWithTimeout(FiveSecondMethod, 6000);

        //try the five second method with a 4 second timeout
        //this will throw a timeout exception
        CallWithTimeout(FiveSecondMethod, 4000);
    }

    static void FiveSecondMethod()
    {
        Thread.Sleep(5000);
    }

The static method doing the work:

    static void CallWithTimeout(Action action, int timeoutMilliseconds)
    {
        Thread threadToKill = null;
        Action wrappedAction = () =>
        {
            threadToKill = Thread.CurrentThread;
            try
            {
                action();
            }
            catch(ThreadAbortException ex){
               Thread.ResetAbort();// cancel hard aborting, lets to finish it nicely.
            }
        };

        IAsyncResult result = wrappedAction.BeginInvoke(null, null);
        if (result.AsyncWaitHandle.WaitOne(timeoutMilliseconds))
        {
            wrappedAction.EndInvoke(result);
        }
        else
        {
            threadToKill.Abort();
            throw new TimeoutException();
        }
    }

}
Milad
  • 17
  • 1
  • 9
TheSoftwareJedi
  • 34,421
  • 21
  • 109
  • 151
  • 3
    Why the catch(ThreadAbortException)? AFAIK you cannot really catch a ThreadAbortException (it will be rethrown after when the catch block is left). – csgero Nov 18 '08 at 16:57
  • You my friend, are absolutely correct. We can just ignore it in this case and let the thread die. The pool will be replenished, but this is something to take note of if this is expected to happen a lot. This *CAN BE* a performance issue. – TheSoftwareJedi Nov 18 '08 at 17:06
  • Just as an aside, I'm not sure how this works in .NET. In java you'd have to declare the Thread object as final which would prevent it's mutating. Somehow the CLR geniuses made this work. Kudos. – TheSoftwareJedi Nov 19 '08 at 00:10
  • 12
    Thread.Abort() is very dangerous to use, It shouldn't be used with regular code, only code that is guaranteed to be safe should be aborted, such as code that is Cer.Safe, uses constrained execution regions and safe handles. It shouldn't be done for any code. – Pop Catalin Nov 19 '08 at 09:04
  • Thread.Abort() is bad. Very bad!!! for unguarded code, see here: http://www.bluebytesoftware.com/blog/PermaLink.aspx?guid=c1898a31-a0aa-40af-871c-7847d98f1641 – Pop Catalin Nov 19 '08 at 09:08
  • 12
    While Thread.Abort() is bad, it is no where near as bad as a process running out of control and using every CPU cycle & byte of memory that the PC has. But you are right to point out the potential problems to anyone else who may think this code is useful. – chilltemp Nov 19 '08 at 15:37
  • 1
    Yes, Thread.Abort is bad, but sometimes other outcomes are worse. – TheSoftwareJedi Dec 01 '08 at 15:33
  • 1
    Shouldn't there be a result.AsyncWaitHandle.Close(); to handle the cleanup? – BozoJoe Feb 23 '09 at 18:53
  • 24
    I can't believe this is the accepted answer, someone must not be reading the comments here, or the answer was accepted before the comments and that person does not check his replies page. Thread.Abort is not a solution, it's just another problem you need to solve! – Lasse V. Karlsen May 15 '09 at 15:21
  • 18
    You are the one not reading the comments. As chilltemp says above, he's calling code that he has NO control over - and wants it to abort. He has no option other than Thread.Abort() if he wants this to run within his process. You are right that Thread.Abort is bad - but like chilltemp says, other things are worse! – TheSoftwareJedi May 15 '09 at 15:55
  • @TheSoftwareJedi, if you abort a thread the next best thing you should do is unload the application domain the thread was running in. – Pop Catalin Oct 21 '09 at 15:31
  • 1
    @Pop Feel free to edit the code and add that feature! Having previously worked with @chilltemp, I know what he's up against here. There is no option he has aside from Abort. – TheSoftwareJedi Oct 21 '09 at 18:32
  • 2
    @TheSoftwareJedi: This is old post but I have a point. You are assuming that the BeginInvoke work item will immediately be picked up by a ThreadPool thread. That assumption will break with NullReferenceException at line threadToKill.Abort(). This will happen when there is a load (more items getting queued then it could process) on ThreadPool and your result.AsyncWaitHandle.WaitOne(timeoutMilliseconds)) getting timed out before your item being picked by ThreadPool. The variable threadToKill remains uninitialized in that case. – Anand Patel Dec 20 '11 at 16:14
  • The CallWithTimeout method takes in an Action, however, a static method is passed in. How does this work? I am trying to implement this atm ... – Karan Apr 13 '12 at 15:33
  • Lasse V. Karlsen vs @TheSoftwareJedi comments? Avoiding Thread.Abort is always a good idea. Avoiding it on a thread you did not create is even better. How To Stop a Thread in .NET (and Why Thread.Abort is Evil) http://www.interact-sw.co.uk/iangblog/2004/11/12/cancellation – Kiquenet Apr 15 '13 at 07:44
  • Is there an adaptation of this which supports parameters? – deed02392 Jul 02 '13 at 09:34
  • Used successfully with anonymous delegate: CallWithTimeout(delegate() { // some code...; }, 15 * 1000); – Stefano Aug 09 '13 at 07:19
  • Is it safe to assume `threadToKill` won't be optimized by the compiler, since it isn't volatile? (I know locals can't be volatile) – fjsj Jul 30 '14 at 16:37
  • Yes, it is safe. However, please recognize that this leaves your program in a unclean state and all attempts should be used to restart it. This is a edge case solution. See @Lasse V. Karlsen comment above. – TheSoftwareJedi Aug 04 '14 at 12:59
  • Also see @Anand Patel comment above. You might want to adjust this to use an explicit thread instead of the threadpool. There is a possibility that your thread never starts. – TheSoftwareJedi Aug 04 '14 at 13:00
73

We are using code like this heavily in production:

var result = WaitFor<Result>.Run(1.Minutes(), () => service.GetSomeFragileResult());

Implementation is open-sourced, works efficiently even in parallel computing scenarios and is available as a part of Lokad Shared Libraries

/// <summary>
/// Helper class for invoking tasks with timeout. Overhead is 0,005 ms.
/// </summary>
/// <typeparam name="TResult">The type of the result.</typeparam>
[Immutable]
public sealed class WaitFor<TResult>
{
    readonly TimeSpan _timeout;

    /// <summary>
    /// Initializes a new instance of the <see cref="WaitFor{T}"/> class, 
    /// using the specified timeout for all operations.
    /// </summary>
    /// <param name="timeout">The timeout.</param>
    public WaitFor(TimeSpan timeout)
    {
        _timeout = timeout;
    }

    /// <summary>
    /// Executes the spcified function within the current thread, aborting it
    /// if it does not complete within the specified timeout interval. 
    /// </summary>
    /// <param name="function">The function.</param>
    /// <returns>result of the function</returns>
    /// <remarks>
    /// The performance trick is that we do not interrupt the current
    /// running thread. Instead, we just create a watcher that will sleep
    /// until the originating thread terminates or until the timeout is
    /// elapsed.
    /// </remarks>
    /// <exception cref="ArgumentNullException">if function is null</exception>
    /// <exception cref="TimeoutException">if the function does not finish in time </exception>
    public TResult Run(Func<TResult> function)
    {
        if (function == null) throw new ArgumentNullException("function");

        var sync = new object();
        var isCompleted = false;

        WaitCallback watcher = obj =>
            {
                var watchedThread = obj as Thread;

                lock (sync)
                {
                    if (!isCompleted)
                    {
                        Monitor.Wait(sync, _timeout);
                    }
                }
                   // CAUTION: the call to Abort() can be blocking in rare situations
                    // http://msdn.microsoft.com/en-us/library/ty8d3wta.aspx
                    // Hence, it should not be called with the 'lock' as it could deadlock
                    // with the 'finally' block below.

                    if (!isCompleted)
                    {
                        watchedThread.Abort();
                    }
        };

        try
        {
            ThreadPool.QueueUserWorkItem(watcher, Thread.CurrentThread);
            return function();
        }
        catch (ThreadAbortException)
        {
            // This is our own exception.
            Thread.ResetAbort();

            throw new TimeoutException(string.Format("The operation has timed out after {0}.", _timeout));
        }
        finally
        {
            lock (sync)
            {
                isCompleted = true;
                Monitor.Pulse(sync);
            }
        }
    }

    /// <summary>
    /// Executes the spcified function within the current thread, aborting it
    /// if it does not complete within the specified timeout interval.
    /// </summary>
    /// <param name="timeout">The timeout.</param>
    /// <param name="function">The function.</param>
    /// <returns>result of the function</returns>
    /// <remarks>
    /// The performance trick is that we do not interrupt the current
    /// running thread. Instead, we just create a watcher that will sleep
    /// until the originating thread terminates or until the timeout is
    /// elapsed.
    /// </remarks>
    /// <exception cref="ArgumentNullException">if function is null</exception>
    /// <exception cref="TimeoutException">if the function does not finish in time </exception>
    public static TResult Run(TimeSpan timeout, Func<TResult> function)
    {
        return new WaitFor<TResult>(timeout).Run(function);
    }
}

This code is still buggy, you can try with this small test program:

      static void Main(string[] args) {

         // Use a sb instead of Console.WriteLine() that is modifying how synchronous object are working
         var sb = new StringBuilder();

         for (var j = 1; j < 10; j++) // do the experiment 10 times to have chances to see the ThreadAbortException
         for (var ii = 8; ii < 15; ii++) {
            int i = ii;
            try {

               Debug.WriteLine(i);
               try {
                  WaitFor<int>.Run(TimeSpan.FromMilliseconds(10), () => {
                     Thread.Sleep(i);
                     sb.Append("Processed " + i + "\r\n");
                     return i;
                  });
               }
               catch (TimeoutException) {
                  sb.Append("Time out for " + i + "\r\n");
               }

               Thread.Sleep(10);  // Here to wait until we get the abort procedure
            }
            catch (ThreadAbortException) {
               Thread.ResetAbort();
               sb.Append(" *** ThreadAbortException on " + i + " *** \r\n");
            }
         }

         Console.WriteLine(sb.ToString());
      }
   }

There is a race condition. It is clearly possible that a ThreadAbortException gets raised after the method WaitFor<int>.Run() is being called. I didn't find a reliable way to fix this, however with the same test I cannot repro any problem with the TheSoftwareJedi accepted answer.

enter image description here

Patrick from NDepend team
  • 13,237
  • 6
  • 61
  • 92
Rinat Abdullin
  • 23,036
  • 8
  • 57
  • 80
  • 3
    This what I implemented, It can handle parameters and return value, which I prefer and needed. Thanks Rinat – Gabriel Mongeon Jul 12 '10 at 20:11
  • 2
    Just an attribute we use to mark immutable classes (immutability is verified by Mono Cecil in unit tests) – Rinat Abdullin Feb 01 '11 at 16:47
  • Great stuff! Works really well. – Luke Belbina Apr 07 '11 at 06:32
  • 9
    This is a deadlock waiting to happen (I’m surprised you haven’t observed it yet). Your call to watchedThread.Abort() is inside a lock, which also needs to be acquired in the finally block. This means while the finally block is waiting for the lock (because the watchedThread has it between Wait() returning and Thread.Abort()), the watchedThread.Abort() call will also block indefinitely waiting for the finally to finish (which it never will). Therad.Abort() can block if a protected region of code is running - causing deadlocks, see - http://msdn.microsoft.com/en-us/library/ty8d3wta.aspx – trickdev Apr 12 '11 at 10:10
  • trickdev, thank you for the comment. I will forward it to the developers responsible for this code. – Rinat Abdullin Apr 14 '11 at 19:05
  • 1
    trickdev, thanks a lot. For some reason, deadlock occurrence appears to be very infrequent, but we have fixed the code nonetheless :-) – Joannes Vermorel Apr 14 '11 at 19:54
  • 1
    How can I call void type functions? – Nime Cloud Aug 10 '11 at 11:03
  • What about 1.Minutes()? Where Minutes() code? what is [Immutable]? What aobut TimeoutException ? Better TryExecute like http://stackoverflow.com/a/1370891/206730 – Kiquenet Apr 12 '13 at 09:51
  • @Kiquenet the Minutes() function is extension method on the int class. See here for example: http://leecampbell.blogspot.fi/2008/11/how-c-30-helps-you-with-creating-dsl.html – Juha Palomäki Oct 03 '13 at 22:22
  • Is it safe to assume isCompleted won't be optimized by the compiler, since it isn't volatile? – fjsj Jul 30 '14 at 16:35
  • 1
    @RinatAbdullin This is really great. I found this in our codebase attributed to you; I imagine it's copied and pasted into a lot of codebases! How would you feel if I were to turn this into a trivial nuget package so that others can use it without copypaste-ing? – Brondahl Nov 06 '15 at 09:25
  • 1
    Maybe use `TimeSpan.FromMinutes(1)` to reduce confusion? Perhaps remove the `[Immutable]` as well. – Hugh Jeffner Mar 29 '16 at 16:58
15

Well, you could do things with delegates (BeginInvoke, with a callback setting a flag - and the original code waiting for that flag or timeout) - but the problem is that it is very hard to shut down the running code. For example, killing (or pausing) a thread is dangerous... so I don't think there is an easy way to do this robustly.

I'll post this, but note it is not ideal - it doesn't stop the long-running task, and it doesn't clean up properly on failure.

    static void Main()
    {
        DoWork(OK, 5000);
        DoWork(Nasty, 5000);
    }
    static void OK()
    {
        Thread.Sleep(1000);
    }
    static void Nasty()
    {
        Thread.Sleep(10000);
    }
    static void DoWork(Action action, int timeout)
    {
        ManualResetEvent evt = new ManualResetEvent(false);
        AsyncCallback cb = delegate {evt.Set();};
        IAsyncResult result = action.BeginInvoke(cb, null);
        if (evt.WaitOne(timeout))
        {
            action.EndInvoke(result);
        }
        else
        {
            throw new TimeoutException();
        }
    }
    static T DoWork<T>(Func<T> func, int timeout)
    {
        ManualResetEvent evt = new ManualResetEvent(false);
        AsyncCallback cb = delegate { evt.Set(); };
        IAsyncResult result = func.BeginInvoke(cb, null);
        if (evt.WaitOne(timeout))
        {
            return func.EndInvoke(result);
        }
        else
        {
            throw new TimeoutException();
        }
    }
Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • 2
    I'm perfectly happy killing something that's gone rouge on me. Its still better than letting it eat CPU cycles until the next reboot (this is part of a windows service). – chilltemp Nov 18 '08 at 16:09
  • @Marc : I am great fan of yours. But, this time, I wonder, why u did not use result.AsyncWaitHandle as mentioned by TheSoftwareJedi. Any benefit of using ManualResetEvent over AsyncWaitHandle? – Anand Patel Dec 19 '11 at 17:30
  • 1
    @Anand well, this was some years ago so I can't answer from memory - but "easy to understand" counts for a lot in threaded code – Marc Gravell Dec 19 '11 at 18:28
13

Some minor changes to Pop Catalin's great answer:

  • Func instead of Action
  • Throw exception on bad timeout value
  • Calling EndInvoke in case of timeout

Overloads have been added to support signaling worker to cancel execution:

public static T Invoke<T> (Func<CancelEventArgs, T> function, TimeSpan timeout) {
    if (timeout.TotalMilliseconds <= 0)
        throw new ArgumentOutOfRangeException ("timeout");

    CancelEventArgs args = new CancelEventArgs (false);
    IAsyncResult functionResult = function.BeginInvoke (args, null, null);
    WaitHandle waitHandle = functionResult.AsyncWaitHandle;
    if (!waitHandle.WaitOne (timeout)) {
        args.Cancel = true; // flag to worker that it should cancel!
        /* •————————————————————————————————————————————————————————————————————————•
           | IMPORTANT: Always call EndInvoke to complete your asynchronous call.   |
           | http://msdn.microsoft.com/en-us/library/2e08f6yc(VS.80).aspx           |
           | (even though we arn't interested in the result)                        |
           •————————————————————————————————————————————————————————————————————————• */
        ThreadPool.UnsafeRegisterWaitForSingleObject (waitHandle,
            (state, timedOut) => function.EndInvoke (functionResult),
            null, -1, true);
        throw new TimeoutException ();
    }
    else
        return function.EndInvoke (functionResult);
}

public static T Invoke<T> (Func<T> function, TimeSpan timeout) {
    return Invoke (args => function (), timeout); // ignore CancelEventArgs
}

public static void Invoke (Action<CancelEventArgs> action, TimeSpan timeout) {
    Invoke<int> (args => { // pass a function that returns 0 & ignore result
        action (args);
        return 0;
    }, timeout);
}

public static void TryInvoke (Action action, TimeSpan timeout) {
    Invoke (args => action (), timeout); // ignore CancelEventArgs
}
George Tsiokos
  • 1,890
  • 21
  • 31
  • Invoke(e => { // ... if (error) e.Cancel = true; return 5; }, TimeSpan.FromSeconds(5)); – George Tsiokos Apr 04 '11 at 21:07
  • 1
    It's worth pointing out that in this answer the 'timed out' method is left running unless it can be modified to politely choose to exit when flagged with 'cancel'. – David Eison Sep 30 '11 at 04:42
  • David, that's what the CancellationToken type (.NET 4.0) was specifically created to address. In this answer, I used CancelEventArgs so the worker could poll args.Cancel to see if it should exit, although this should be re-implemented with the CancellationToken for .NET 4.0. – George Tsiokos Dec 21 '11 at 15:55
  • A usage note on this that confused me for a little while: You need two try/catch blocks if your Function/Action code may throw an exception after timeout. You need one try/catch around the call to Invoke to catch TimeoutException. You need a second inside your Function/Action to capture and swallow/log any exception that may occur after your timeout throws. Otherwise the app will terminate with an unhandled exception (my use case is ping testing a WCF connection on a tighter timeout than specified in app.config) – fiat Oct 19 '12 at 00:20
  • Absolutely - since the code inside the function/action can throw, it must be inside a try/catch. Per convention, these methods do not attempt to try/catch the function/action. It's a bad design to catch and throw away the exception. As with all asynchronous code it's up to the user of the method to try/catch. – George Tsiokos Oct 19 '12 at 14:56
10

This is how I'd do it:

public static class Runner
{
    public static void Run(Action action, TimeSpan timeout)
    {
        IAsyncResult ar = action.BeginInvoke(null, null);
        if (ar.AsyncWaitHandle.WaitOne(timeout))
            action.EndInvoke(ar); // This is necesary so that any exceptions thrown by action delegate is rethrown on completion
        else
            throw new TimeoutException("Action failed to complete using the given timeout!");
    }
}
Pop Catalin
  • 61,751
  • 23
  • 87
  • 115
7

I just knocked this out now so it might need some improvement, but will do what you want. It is a simple console app, but demonstrates the principles needed.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;


namespace TemporalThingy
{
    class Program
    {
        static void Main(string[] args)
        {
            Action action = () => Thread.Sleep(10000);
            DoSomething(action, 5000);
            Console.ReadKey();
        }

        static void DoSomething(Action action, int timeout)
        {
            EventWaitHandle waitHandle = new EventWaitHandle(false, EventResetMode.ManualReset);
            AsyncCallback callback = ar => waitHandle.Set();
            action.BeginInvoke(callback, null);

            if (!waitHandle.WaitOne(timeout))
                throw new Exception("Failed to complete in the timeout specified.");
        }
    }

}
Jason Jackson
  • 17,016
  • 8
  • 49
  • 74
2

What about using Thread.Join(int timeout)?

public static void CallWithTimeout(Action act, int millisecondsTimeout)
{
    var thread = new Thread(new ThreadStart(act));
    thread.Start();
    if (!thread.Join(millisecondsTimeout))
        throw new Exception("Timed out");
}
  • 1
    That would notify the calling method of a problem, but not abort the offending thread. – chilltemp Jan 11 '10 at 21:27
  • 1
    I'm not sure that's correct. It's not clear from the documentation what happens to the worker thread when the Join timeout elapses. – Matthew Lowe Aug 18 '10 at 14:01