3
\$\begingroup\$

I have an interface named ICodeRunner, defined like this:

public interface ICodeRunner
{
        ExecutionResult Run(string code);
}

I have more classes that implement ICodeRunner, but regardless of which one is used, in some contexts only I have to impose a time limit on the execution of the Run method. This is a regular timeout: "if it's not done in x seconds, throw an exception."

To achieve this, I have created a class called CodeRunnerTimeLimitDecorator that can wrap any class implementing ICodeRunner and provide this functionality.

This is how the class is implemented:

public class CodeRunnerTimeLimitDecorator : ICodeRunner
{
        private readonly ICodeRunner _wrapped;
        private readonly int _timeoutInSeconds;

        public CodeRunnerTimeLimitDecorator(ICodeRunner wrapped, int timeoutInSeconds)
        {
                _wrapped = wrapped;
                _timeoutInSeconds = timeoutInSeconds;
        }

        public ExecutionResult Run(string code)
        {
                var executionResult = new ExecutionResult();

                var t = new Thread(() => executionResult = _wrapped.Run(code));

                t.Start();

                var completed = t.Join(_timeoutInSeconds * 1000);

                if (completed)
                        return executionResult;

                throw new ApplicationException(
                        string.Format("Code took longer than {0} seconds to run, so it was aborted.", _timeoutInSeconds));
        }
}

While I welcome any advice about how to make this code better, I'm particularly interested in comments/improvements regarding the time limit mechanism.

Update:

More specifically, the questions are:

  1. Is there an easier/cleaner way to do it?

  2. Would it be OK if I called t.Abort() in case the timeout had elapsed*?

*I do not control the process that goes on inside _wrapped.Run(code), so I cannot implement Task Cancellation or similar techniques.

\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

This is actually a stab in the dark but could you use the .NET 4 System.Threading.Task object for this. Something like:

var task = Task.Factory.StartNew(() => _wrapped.Run(code) });

// Wait returns true if the task finished within the allocated timeframe
// http://msdn.microsoft.com/en-us/library/dd270644.aspx
if(task.Wait(_timeoutInSeconds * 1000))
{
   return executionResult;
}
else
{
   throw new ApplicationException(string.Format("Code took longer than {0} seconds to run, so it was aborted.", _timeoutInSeconds));
}
\$\endgroup\$
5
  • \$\begingroup\$ This definitely gains in terms of conciseness and code clarity, so +1! I am also looking for a advice on aborting the thread, in case it has exceeded the timeout. \$\endgroup\$ Jun 6, 2012 at 11:12
  • \$\begingroup\$ @w0lf there is a CancellationToken that can be used with the TPL which has been shown above. msdn.microsoft.com/en-us/library/dd997396.aspx or see stackoverflow.com/questions/4783865/… \$\endgroup\$ Jun 6, 2012 at 12:30
  • \$\begingroup\$ @TrevorPilley Thanks for this clarification; I just realized I did not properly formulate the question. Please see my edit. \$\endgroup\$ Jun 6, 2012 at 12:48
  • \$\begingroup\$ @w0lf I don't think you should really be either 'newing' up a Thread or calling Thread.Abort(), have a look into ThreadPool.QueueUserWorkItem instead \$\endgroup\$ Jun 6, 2012 at 14:58
  • \$\begingroup\$ @W0lf ah, my bad. I'll be interested to see what others say! \$\endgroup\$
    – dreza
    Jun 6, 2012 at 20:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.