-
Notifications
You must be signed in to change notification settings - Fork 127
Safely executing user provided scripts
Imagine executing a JavaScript like this in Jurassic:
for (;;)If you run such an endless loop using ScriptEngine.Execute(String code), the method would never return.
Jurassic does not have an built-in way to limit the execution time of a script. Because Jurassic compiles JavaScript methods into IL code, there is no easy way to provide a timeout functionality without affecting performance.
However, it is possible to use Thread.Abort() to raise a ThreadAbortException in the thread that executes the script. For example, you could run ScriptEngine.Execute() in a new Thread, and in the current Thread, call thread.Abort() if the other thread does not complete after a specific time. You can also use the following helper class for a simple way to apply a timeout to script execution.
If you don't want to run the script in a different Thread and don't want to deal with ThreadAbortExceptions, and maybe your script can call .NET methods which should not be aborted by a ThreadAbortException (only the script code should), you can use the following ScriptTimeoutHelper class that does the management under the hood and provides a simple way to apply a timeout to script execution.
This assumes you are targeting .NET 4.5 or higher and are using Visual Studio 2015 or higher.
using System;
using System.Runtime.ExceptionServices;
using System.Threading;
using System.Threading.Tasks;
namespace JurassicTimeoutHelper
{
/// <summary>
/// Allows to limit execution time of a jurassic script by internally using the
/// technique of aborting a thread.
/// </summary>
public class ScriptTimeoutHelper
{
private HandlerState currentState;
public ScriptTimeoutHelper()
{
}
/// <summary>
/// Runs the specified <see cref="Action"/>, applying the given timeout.
/// When the handler times out, a <see cref="ThreadAbortException"/> is
/// raised in the current thread to break. However, this is managed
/// internally so it does not affect the caller of this method (i.e. it is
/// ensured that a ThreadAbortException does not flow through this method
/// or raised after this method returns).
/// </summary>
/// <exception cref="TimeoutException">Thrown when the handler times
/// out.</exception>
/// <param name="handler"></param>
/// <param name="timeout"></param>
public void RunWithTimeout(Action handler, int timeout)
{
if (currentState != null)
throw new InvalidOperationException(
$"Cannot recursively call {nameof(RunWithTimeout)}.");
if (handler == null)
throw new ArgumentException(nameof(handler));
ExceptionDispatchInfo catchedException = null;
using (var state = currentState =
new HandlerState(Thread.CurrentThread))
{
/* Start a monitoring task that may abort the current thread after
* the specified time limit.
* Note that the task will start immediately. Therefore we need to
* ensure the task does not abort the thread until we entered the
* try clause; otherwise the ThreadAbortException might fly through
* the caller of this method. To ensure this, the monitoring task
* waits until we release the semaphore the first time before
* actually waiting for the specified time.
*/
using (var monitoringTask = Task.Run(async () =>
await RunMonitoringTask(state, timeout)))
{
try
{
bool waitForAbortException;
try
{
// Allow the monitoring task to begin by releasing the
// semaphore the first time.
// Do this in a finally block to ensure if this thread
// is aborted by other code, the semaphore is still
// released.
try { }
finally
{
state.WaitSemaphore.Release();
}
// Execute the handler.
handler();
}
catch (Exception ex) when (!(ex is ThreadAbortException))
{
/* Need to catch all exceptions (except our own
* ThreadAbortException) because we may wait for a
* ThreadAbortException to be thrown which is not
* possible in a finally handler.
*/
catchedException = ExceptionDispatchInfo.Capture(ex);
}
finally
{
/* Indicate that the handler is completed, and check
* if we need to wait for the ThreadAbortException.
* This is done in a finally handler to ensure when
* other code wants to abort this thread, the thread
* actually will abort as expected but we still can
* notify the monitoring task that we already returned.
*/
lock (state)
{
state.IsExited = true;
waitForAbortException =
state.AbortState == AbortState.IsAborting;
if (state.AbortState == AbortState.None)
{
// If the monitoring task did not do anything
// yet, allow it to complete immediately.
state.WaitSemaphore.Release();
}
}
}
if (waitForAbortException)
{
/* The monitoring task indicated that it will abort our
* thread (but the ThreadAbortException did not yet
* occur), so we need to wait for the
* ThreadAbortException.
* This wait is needed because otherwise we may return
* too early (and in the finally block we wait for the
* monitoring task, causing a deadlock).
*/
Thread.Sleep(Timeout.Infinite);
}
}
catch (ThreadAbortException ex)
when (ex.ExceptionState == state)
{
// Reset the abort.
Thread.ResetAbort();
// Indicate that the timeout has been exceeded.
throw new TimeoutException();
}
finally
{
// Wait for the monitoring task to complete.
monitoringTask.Wait();
currentState = null;
}
}
}
// Check if we need to rethrow a catched exception (preserving the
// original stacktrace).
if (catchedException != null)
catchedException.Throw();
}
private async Task RunMonitoringTask(HandlerState state, int timeout)
{
// Wait until the handler thread entered the try-block.
// Use a synchronous wait because we expect this to be a very short
// period of time.
state.WaitSemaphore.Wait();
// Now asynchronously wait until the specified time has passed or the
// semaphore has been released. In the latter case there is no need to
// call AbortExecution().
bool completed = await state.WaitSemaphore.WaitAsync(timeout);
// Abort the handler thread.
if (!completed)
AbortExecution(state);
}
private void AbortExecution(HandlerState state)
{
bool canAbort;
lock (state)
{
if (state.IsExited)
{
// The handler has already exited.
return;
}
// Check if we can call Thread.Abort() or if the handler thread is
// currently in a critical section and needs to abort himself when
// leaving the critical section.
canAbort = !state.IsCriticalSection;
state.AbortState = canAbort ? AbortState.IsAborting
: AbortState.ShouldAbort;
}
if (canAbort)
{
/* The handler thread is not in a critical section so we can
* directly abort it.
* This needs to be done outside of the lock because Abort() could
* block if the thread is currently in a finally handler (and
* trying to lock on the state object), which could lead to a
* deadlock.
*/
state.HandlerThread.Abort(state);
}
}
/// <summary>
/// Notifies this class that the handler thread is entering a critical
/// section in which aborting the thread could corrupt the system's state.
/// This means aborting the thread is deferred until leaving the critical
/// section.
/// Note you must call <see cref="ExitCriticalSection"/> once the thread
/// left the critical section (ideally in a finally block).
/// </summary>
public void EnterCriticalSection()
{
if (currentState == null)
throw new InvalidOperationException();
bool waitForAbortException;
lock (currentState)
{
if (Thread.CurrentThread != currentState.HandlerThread
|| currentState.IsCriticalSection)
throw new InvalidOperationException();
currentState.IsCriticalSection = true;
waitForAbortException =
currentState.AbortState == AbortState.IsAborting;
}
if (waitForAbortException)
{
// The monitoring task indicated that it will abort our thread, so
// we need to wait for the ThreadAbortException.
Thread.Sleep(Timeout.Infinite);
}
}
public void ExitCriticalSection()
{
if (currentState == null)
throw new InvalidOperationException();
bool shouldAbort;
lock (currentState)
{
if (Thread.CurrentThread != currentState.HandlerThread
|| !currentState.IsCriticalSection)
throw new InvalidOperationException();
currentState.IsCriticalSection = false;
shouldAbort = currentState.AbortState == AbortState.ShouldAbort;
}
if (shouldAbort)
{
// The monitoring task indicated that it wanted to abort our
// thread while we were in a critical section, so we need to abort
// ourselves.
Thread.CurrentThread.Abort(currentState);
}
}
private enum AbortState
{
/// <summary>
/// Indicates that the monitoring task has not yet done any action.
/// </summary>
None = 0,
/// <summary>
/// Indicates that the monitoring task is about to abort the handler
/// thread.
/// </summary>
IsAborting = 1,
/// <summary>
/// Indicates that the monitoring task wanted to abort the handler
/// thread but the handler thread was in a critical section, and needs
/// to abort itself when leaving the critical section.
/// </summary>
ShouldAbort = 2
}
private class HandlerState : IDisposable
{
public Thread HandlerThread { get; }
public SemaphoreSlim WaitSemaphore { get; } = new SemaphoreSlim(0);
public bool IsCriticalSection { get; set; }
/// <summary>
/// Indicates if the handler is already completed (so there's no need
/// to abort the thread).
/// This flag is set by the handler thread.
/// </summary>
public bool IsExited { get; set; }
/// <summary>
/// Indicates that the wait task wanted to abort the handler thread but
/// the handler thread was in a critical section, and needs to abort
/// itself when leaving the critical section.
/// This flag is set by the wait task.
/// </summary>
public AbortState AbortState { get; set; }
public HandlerState(Thread handlerThread)
{
this.HandlerThread = handlerThread;
}
~HandlerState()
{
Dispose(false);
}
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
protected void Dispose(bool disposing)
{
if (disposing)
{
WaitSemaphore.Dispose();
}
}
}
}
}This class starts an async Task which waits for the timeout using a SemaphoreSlim (so that there is no separate thread blocked by waiting for the timeout). RunWithTimeout() catches the ThreadAbortException and resets the abort to ensure the thread can continue normally after handling the abort.
Now, in your code, instead of writing
var engine = new ScriptEngine();
engine.Execute("for (;;);");you can write:
var timeoutHelper = new ScriptTimeoutHelper();
var engine = new ScriptEngine();
timeoutHelper.RunWithTimeout(() => engine.Execute("for (;;);"), 1000);You will now get a TimeoutException if the JavaScript did not finish within 1 second.
Note: After the TimeoutException occured, you should not continue to use the ScriptEngine object as it may be in an inconsistent/unusable state. Instead, create a new ScriptEngine to run further scripts.
Using the ScriptTimeoutHelper class means that the code that is currently executing within the RunWithTimeout can be aborted by a ThreadAbortException. This can be a problem if your scripts are able to call back a .NET method, but the .NET method should not be aborted in the middle of its execution, e.g. because it would leave the system in an inconsistent state.
To handle this problem, call ScriptTimeoutHelper.EnterCriticalSection() once your .NET method is called, and call ScriptTimeoutHelper.ExitCriticalSection() (ideally in a finally block) once your .NET method returns. This ensures a ThreadAbortException is delayed until the critical section is left.
Example:
Imagine you have an API class with a method, that increments three variables:
class Api
{
public int a = 0;
public long b = 0;
public double c = 0;
// Increments all three counters.
public void Increment()
{
a++;
// Loop to simulate other work
for (int i = 0; i < 1000000; i++) ;
b++;
for (int i = 0; i < 1000000; i++) ;
c++;
}
}Calling Api.Increment() should increment all three counters, so it should not happen that a = 5 and b = 6 after the method returns - all three counters should have the same value as long as the int variable doesn't overflow. As long as you don't abort a thread (or e.g. a OutOfMemoryException occurs), you can expect this to work.
Now imagine that you want to allow a JavaScript to call this method, but limit the script's execution time:
class ScriptApi : ObjectInstance
{
private Api api;
public ScriptApi(ScriptEngine engine, Api api)
: base(engine)
{
this.api = api;
PopulateFunctions();
}
[JSFunction(Name = "increment")]
public void Increment() => api.Increment();
}
class Program
{
private static Api api = new Api();
static void Main()
{
var timeoutHelper = new ScriptTimeoutHelper();
var engine = new ScriptEngine();
var scriptApi = new ScriptApi(engine, api);
engine.SetGlobalValue("api", scriptApi);
try
{
timeoutHelper.RunWithTimeout(() => engine.Execute(@"
while (true) {
// Call a .NET API method.
api.increment();
}
"), 1000);
}
catch (TimeoutException)
{
Console.WriteLine("Script timed-out.");
// Note: The ScriptEngine object might now be in an
// inconsistent/unable state, so you should create a new
// ScriptEngine object to execute further scripts.
}
Console.WriteLine($"Result: a = {api.a}, b = {api.b}, c = {api.c}");
Console.ReadKey();
}
}If you run this program a few times, you will notice that sometimes a, b and c will have different values. This is because when the thread is aborted, it could either be in script code (Lightweight Function) or in the Api.Increment() method.
To prevent the thread from being aborted while being in the Api.Increment() method, you will need to modify the ScriptApi class in the following way:
class ScriptApi : ObjectInstance
{
private Api api;
private ScriptTimeoutHelper timeoutHelper;
public ScriptApi(ScriptEngine engine, Api api,
ScriptTimeoutHelper timeoutHelper)
: base(engine)
{
this.api = api;
this.timeoutHelper = timeoutHelper;
PopulateFunctions();
}
[JSFunction(Name = "increment")]
public void Increment()
{
timeoutHelper.EnterCriticalSection();
try
{
// Call critical code which must not be aborted here.
api.Increment();
}
finally
{
timeoutHelper.ExitCriticalSection();
}
}
}Now, you can see that every time the script is aborted, all three counters will have the same value. The script will still be aborted after about 1 second, provided that the "critical" method (Api.Increment()) does not take very long to execute.