Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experimental - Task to Promise conversion #1567

Merged
merged 6 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions Jint.Tests/Runtime/AwaitTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,72 @@ public void AwaitPropagationAgainstPrimitiveValue()
result = result.UnwrapIfPromise();
Assert.Equal("1", result);
}

[Fact]
public void ShouldTaskConvertedToPromiseInJS()
{
Engine engine = new();
engine.SetValue("callable", Callable);
var result = engine.Evaluate("callable().then(x=>x*2)");
result = result.UnwrapIfPromise();
Assert.Equal(2, result);

static async Task<int> Callable()
{
await Task.Delay(10);
Assert.True(true);
return 1;
}
}

[Fact]
public void ShouldTaskCatchWhenCancelled()
{
Engine engine = new();
CancellationTokenSource cancel = new();
cancel.Cancel();
engine.SetValue("token", cancel.Token);
engine.SetValue("callable", Callable);
engine.SetValue("assert", new Action<bool>(Assert.True));
var result = engine.Evaluate("callable(token).then(_ => assert(false)).catch(_ => assert(true))");
result = result.UnwrapIfPromise();
static async Task Callable(CancellationToken token)
{
await Task.FromCanceled(token);
}
}

[Fact]
public void ShouldTaskCatchWhenThrowError()
{
Engine engine = new();
engine.SetValue("callable", Callable);
engine.SetValue("assert", new Action<bool>(Assert.True));
var result = engine.Evaluate("callable().then(_ => assert(false)).catch(_ => assert(true))");

static async Task Callable()
{
await Task.Delay(10);
throw new Exception();
}
}

[Fact]
public void ShouldTaskAwaitCurrentStack()
{
//https://github.com/sebastienros/jint/issues/514#issuecomment-1507127509
Engine engine = new();
string log = "";
engine.SetValue("myAsyncMethod", new Func<Task>(async () =>
{
await Task.Delay(1000);
log += "1";
}));
engine.SetValue("myAsyncMethod2", new Action(() =>
{
log += "2";
}));
engine.Execute("async function hello() {await myAsyncMethod();myAsyncMethod2();} hello();");
Assert.Equal("12", log);
}
}
56 changes: 54 additions & 2 deletions Jint/Runtime/Interop/DelegateWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,65 @@ protected internal override JsValue Call(JsValue thisObject, JsValue[] jsArgumen

try
{
return FromObject(Engine, _d.DynamicInvoke(parameters));
var result = _d.DynamicInvoke(parameters);
if (result is not Task task)
{
return FromObject(Engine, result);
}
return ConvertTaskToPromise(task);
}
catch (TargetInvocationException exception)
{
ExceptionHelper.ThrowMeaningfulException(_engine, exception);
ExceptionHelper.ThrowMeaningfulException(Engine, exception);
throw;
}
}

private JsValue ConvertTaskToPromise(Task task)
{
var (promise, resolve, reject) = Engine.RegisterPromise();
task = task.ContinueWith(continuationAction =>
{
if (continuationAction.IsFaulted)
{
reject(FromObject(Engine, continuationAction.Exception));
}
else if (continuationAction.IsCanceled)
{
reject(FromObject(Engine, new ExecutionCanceledException()));
}
else
{
// Special case: Marshal `async Task` as undefined, as this is `Task<VoidTaskResult>` at runtime
// See https://github.com/sebastienros/jint/pull/1567#issuecomment-1681987702
if (Task.CompletedTask.Equals(continuationAction))
{
resolve(FromObject(Engine, JsValue.Undefined));
return;
}

var result = continuationAction.GetType().GetProperty(nameof(Task<object>.Result));
if (result is not null)
{
resolve(FromObject(Engine, result.GetValue(continuationAction)));
}
else
{
resolve(FromObject(Engine, JsValue.Undefined));
}
}
});

Engine.AddToEventLoop(() =>
{
if (!task.IsCompleted)
{
// Task.Wait has the potential of inlining the task's execution on the current thread; avoid this.
((IAsyncResult) task).AsyncWaitHandle.WaitOne();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that JS thread will be blocked until task is resolved?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, when promise needs to resolve, block to js thread until resolved.

I'm not sure but Evalutate function in unit tests, only process the script. It does not block until call RunAvailableContinuations

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think that is the main problem that I encountered with the Task mapping too.

Tasks (plural in general case) can be resolved out of order at any time on any thread. But in JS it is one thread that collects continuations in the order they came in, in other words non blocking at any time. So I would probably advocate to use that as a guiding principal, and to my understanding the current code is not aligned with that, or I'm missing something obv?

Copy link
Contributor Author

@Xicy Xicy Jun 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my understanding the current code is not aligned with that

As per your explanation, yes code does not align. Maybe EventLoop works on another thread that can be closer to working asynchrony.

By the way, I don't have experience with that. If you lead the way, I try to enhance the code.

Edit:
PS: When I thought about works in another thread, It may occurs an error, because the engine does not thread-safe.

}
});

return promise;
}
}
}