0

I require a method that will throw an exception if a function that is passed into it is not completed before X minutes.

This is my original code:

public class Poller : IPoller
    {
        public async Task<bool> RunTimedFunction(string taskName, 
                                 int timeoutMs, 
                                 Func<bool> pollMethod)
        {
             var canPoll = true;
             var returnValue = false;
             var watch = System.Diagnostics.Stopwatch.StartNew();

        var t = Task.Factory.StartNew(() =>
        {
            watch.Start();
            returnValue = pollMethod();
            canPoll = false;
            return returnValue;
        });


        while (canPoll)
        {
            if(watch.ElapsedMilliseconds >= timeoutMs)
                throw new TimeoutException(String.Format("Task: {0} has timed out", taskName));
        }
        await t;

        return returnValue;
    }
}

I can test that it works with the following:

    [Test]
    [ExpectedException(typeof(TimeoutException))]
    public async Task  Poller_PollTimeout()
    {
        var name = "Timeout";
        var timeout = 10;
        var func = new Func<bool>(() =>
        {
            Thread.Sleep(1000);
            return true;
        });
        var t = _poller.Poll(name, timeout, func);

        await t.ContinueWith((task) =>
        {
            if (task.Exception != null)
                throw task.Exception.InnerException;
        });
    }

From suggestions I now have:

public class Poller : IPoller
{
    public async Task<T> RunTimedFunction<T>(string taskName, 
                                             int timeoutMs, 
                                             Func<T> pollMethod)
    {
        var timerTask = Task.Delay(timeoutMs);
        var funcTask = Task.Run(pollMethod);
        var firstFinished = await Task.WhenAny(timerTask, 
                                               funcTask);

        if(firstFinished == timerTask)
            throw new TimeoutException(String.Format("Task: {0} has timed out", taskName));

        return funcTask.Result;
    }
}
  • 1
    `This implementation works as I would like it to ` So you want it to be indeterminate as to whether this method ever returns? It's appropriate for it to never end, even long after the poll method has returned? – Servy Mar 15 '16 at 17:21
  • 1
    Look at my comments here http://stackoverflow.com/a/14019738/1033686 – Matthew Layton Mar 15 '16 at 17:21
  • 1
    i think what you doing is unnecessarily complex – M.kazem Akhgary Mar 15 '16 at 17:21
  • @SeriesOne it's not for profiling purposes it's just for a call to an external api that can take a large amount of time – edwin the duck Mar 15 '16 at 17:31
  • @edwintheduck It still would make way more sense to use `System.Diagnostics.Stopwatch` here. As M.kazem said, this is unnecessarily complex. – Nate Barbettini Mar 15 '16 at 17:32
  • 1
    [You should never use `Task.Factory.StartNew` without passing in a TaskSchedueller](http://blog.stephencleary.com/2013/08/startnew-is-dangerous.html). Use `Task.Run(` instead. – Scott Chamberlain Mar 15 '16 at 17:32
  • @ScottChamberlain I haven't come across that before ty. – edwin the duck Mar 15 '16 at 17:36
  • Sample shows multi-threaded code with unprotected (and possibly cached) access to shared variable - please make sure you have 110% understanding of C#/.Net memory model if you trying to write lock-free code... (I don't understand what you trying to achieve to recommend anything - maybe task with timeout as shown in http://stackoverflow.com/questions/9846615/async-task-whenall-with-timeout) – Alexei Levenkov Mar 15 '16 at 19:50

1 Answers1

3

how about

public async Task<Tuple<int, T>> TimeFunc<T>(
        Func<T> target,
        int timeoutMilliseconds)
{
    var timeoutTask = Task.Delay(timeoutMilliseconds);
    var timer = Stopwatch.StartNew();
    var funcTask = Task.Run(() => target());
    var first = await Task.WhenAny(new[] { timeoutTask, funcTask });
    timer.Stop();

    if (first == timeoutTask)
    {
        throw new TimeoutException();
    }

    return Tuple.Create(timer.ElapsedMilliseconds, funcTask.Result);
}
Jodrell
  • 34,946
  • 5
  • 87
  • 124
  • Your `if(results[1].RanToCompletion)` won't compile because `results` is just a `Task` not a `Task[]`, keep a reference to the array you passed in to the WhenAny and do `if(results == taskArray[1])` instead. – Scott Chamberlain Mar 15 '16 at 17:35
  • @ScottChamberlain got my When mixed up with my Wait, which you definitely shouldn't do. – Jodrell Mar 15 '16 at 17:40
  • This functions notably differently than the OP's code, and he said his works the way that he wants. – Servy Mar 15 '16 at 17:40
  • That said, if you want to actually implement this idiomatically, why are you inventing your own cancellation token system, instead of just using a cancellation token? – Servy Mar 15 '16 at 17:41
  • @Servy because I'm no good at dichotomies, and I'm not trying to be flippant. – Jodrell Mar 15 '16 at 17:42
  • I still would not do `if(timeoutTask.RanToCompletion)` and instead keep the result from `await Task.WhenAny` and do `if(result == timeoutTask)`, otherwise if the func completed but was very close to the timeout time it may appear that it timed out when actually it finished. – Scott Chamberlain Mar 15 '16 at 17:53