Skip to content

Commit b126818

Browse files
ClearScriptLibClearScriptLib
committed
ClearScript 7.0 RC3: Fixed more task-promise conversion issues (GitHub Issue ClearFoundry#198).
1 parent 3fbb65b commit b126818

File tree

13 files changed

+140
-76
lines changed

13 files changed

+140
-76
lines changed

‎ClearScript/Exports/VersionSymbols.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@
77

88
#define CLEARSCRIPT_VERSION_STRING "7.0.0"
99
#define CLEARSCRIPT_VERSION_COMMA_SEPARATED 7,0,0
10-
#define CLEARSCRIPT_VERSION_STRING_INFORMATIONAL "7.0.0-rc2"
10+
#define CLEARSCRIPT_VERSION_STRING_INFORMATIONAL "7.0.0-rc3"
1111
#define CLEARSCRIPT_FILE_FLAGS VS_FF_PRERELEASE

‎ClearScript/JavaScript/IJavaScriptEngine.cs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
1-
using System.Threading.Tasks;
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT license.
3+
4+
using System.Threading.Tasks;
25

36
namespace Microsoft.ClearScript.JavaScript
47
{
58
internal interface IJavaScriptEngine
69
{
710
uint BaseLanguageVersion { get; }
811

9-
void CompletePromiseWithResult<T>(Task<T> task, object resolve, object reject);
10-
void CompletePromise(Task task, object resolve, object reject);
12+
object CreatePromiseForTask<T>(Task<T> task);
13+
object CreatePromiseForTask(Task task);
14+
15+
Task<object> CreateTaskForPromise(ScriptObject promise);
1116
}
1217
}

‎ClearScript/JavaScript/JavaScriptExtensions.cs‎

Lines changed: 8 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ namespace Microsoft.ClearScript.JavaScript
1212
/// </summary>
1313
public static class JavaScriptExtensions
1414
{
15-
private delegate void Executor(object resolve, object reject);
16-
1715
/// <summary>
1816
/// Converts a <see cref="Task{TResult}"/> instance to a
1917
/// <see href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise">promise</see>
@@ -47,11 +45,7 @@ public static object ToPromise<TResult>(this Task<TResult> task, ScriptEngine en
4745
throw new NotSupportedException("The script engine does not support promises");
4846
}
4947

50-
return engine.Script.EngineInternal.createPromise(new Executor((resolve, reject) =>
51-
{
52-
Action<Task<TResult>> continuation = thisTask => javaScriptEngine.CompletePromiseWithResult(thisTask, resolve, reject);
53-
task.ContinueWith(continuation, TaskContinuationOptions.ExecuteSynchronously);
54-
}));
48+
return javaScriptEngine.CreatePromiseForTask(task);
5549
}
5650

5751
/// <summary>
@@ -85,11 +79,7 @@ public static object ToPromise(this Task task, ScriptEngine engine)
8579
throw new NotSupportedException("The script engine does not support promises");
8680
}
8781

88-
return engine.Script.EngineInternal.createPromise(new Executor((resolve, reject) =>
89-
{
90-
Action<Task> continuation = thisTask => javaScriptEngine.CompletePromise(thisTask, resolve, reject);
91-
task.ContinueWith(continuation, TaskContinuationOptions.ExecuteSynchronously);
92-
}));
82+
return javaScriptEngine.CreatePromiseForTask(task);
9383
}
9484

9585
/// <summary>
@@ -104,33 +94,18 @@ public static Task<object> ToTask(this object promise)
10494
MiscHelpers.VerifyNonNullArgument(promise, "promise");
10595

10696
var scriptObject = promise as ScriptObject;
107-
if ((scriptObject == null) || !scriptObject.Engine.Script.EngineInternal.isPromise(promise))
97+
if (scriptObject == null)
10898
{
10999
throw new ArgumentException("The object is not a promise", nameof(promise));
110100
}
111101

112-
var source = new TaskCompletionSource<object>();
113-
114-
Action<object> onResolved = result =>
115-
{
116-
source.SetResult(result);
117-
};
118-
119-
Action<object> onRejected = error =>
102+
var javaScriptEngine = scriptObject.Engine as IJavaScriptEngine;
103+
if ((javaScriptEngine == null) || (javaScriptEngine.BaseLanguageVersion < 6))
120104
{
121-
try
122-
{
123-
scriptObject.Engine.Script.EngineInternal.throwValue(error);
124-
}
125-
catch (Exception exception)
126-
{
127-
source.SetException(exception);
128-
}
129-
};
130-
131-
scriptObject.InvokeMethod("then", onResolved, onRejected);
105+
throw new NotSupportedException("The script engine does not support promises");
106+
}
132107

133-
return source.Task;
108+
return javaScriptEngine.CreateTaskForPromise(scriptObject);
134109
}
135110
}
136111
}

‎ClearScript/Properties/AssemblyInfo.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@
1717
[assembly: ComVisible(false)]
1818
[assembly: AssemblyVersion("7.0.0")]
1919
[assembly: AssemblyFileVersion("7.0.0")]
20-
[assembly: AssemblyInformationalVersion("7.0.0-rc2")]
20+
[assembly: AssemblyInformationalVersion("7.0.0-rc3")]
2121

2222
namespace Microsoft.ClearScript.Properties
2323
{
2424
internal static class ClearScriptVersion
2525
{
2626
public const string Triad = "7.0.0";
27-
public const string Informational = "7.0.0-rc2";
27+
public const string Informational = "7.0.0-rc3";
2828
}
2929
}

‎ClearScript/V8/V8ScriptEngine.cs‎

Lines changed: 69 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ public sealed class V8ScriptEngine : ScriptEngine, IJavaScriptEngine
3030
#region data
3131

3232
private static readonly DocumentInfo initScriptInfo = new DocumentInfo(MiscHelpers.FormatInvariant("{0} [internal]", nameof(V8ScriptEngine)));
33-
[ThreadStatic] private static bool bypassTaskPromiseConversion;
3433

3534
private readonly V8ScriptEngineFlags engineFlags;
3635
private readonly V8ContextProxy proxy;
@@ -288,23 +287,25 @@ function construct() {
288287
return value instanceof savedPromise;
289288
},
290289
291-
completePromiseWithResult: function (task, resolve, reject) {
290+
completePromiseWithResult: function (getResult, resolve, reject) {
292291
try {
293-
resolve(task.Result);
292+
resolve(getResult());
294293
}
295294
catch (exception) {
296295
reject(exception);
297296
}
297+
return undefined;
298298
},
299299
300-
completePromise: function (task, resolve, reject) {
300+
completePromise: function (wait, resolve, reject) {
301301
try {
302-
task.Wait();
302+
wait();
303303
resolve();
304304
}
305305
catch (exception) {
306306
reject(exception);
307307
}
308+
return undefined;
308309
},
309310
310311
throwValue: function (value) {
@@ -1101,6 +1102,26 @@ private void OnContinuationTimer(Timer timer)
11011102
}
11021103
}
11031104

1105+
private object CreatePromise(Action<object, object> executor)
1106+
{
1107+
VerifyNotDisposed();
1108+
var v8Script = (V8ScriptItem)script;
1109+
var v8Internal = (V8ScriptItem)v8Script.GetProperty("EngineInternal");
1110+
return V8ScriptItem.Wrap(this, v8Internal.InvokeMethod(false, "createPromise", executor));
1111+
}
1112+
1113+
private void CompletePromise<T>(Task<T> task, object resolve, object reject)
1114+
{
1115+
Func<T> getResult = () => task.Result;
1116+
Script.EngineInternal.completePromiseWithResult(getResult, resolve, reject);
1117+
}
1118+
1119+
private void CompletePromise(Task task, object resolve, object reject)
1120+
{
1121+
Action wait = task.Wait;
1122+
Script.EngineInternal.completePromise(wait, resolve, reject);
1123+
}
1124+
11041125
#endregion
11051126

11061127
#region ScriptEngine overrides (public members)
@@ -1289,7 +1310,7 @@ internal override object MarshalToScript(object obj, HostItemFlags flags)
12891310
return obj;
12901311
}
12911312

1292-
if (engineFlags.HasFlag(V8ScriptEngineFlags.EnableTaskPromiseConversion) && !bypassTaskPromiseConversion)
1313+
if (engineFlags.HasFlag(V8ScriptEngineFlags.EnableTaskPromiseConversion))
12931314
{
12941315
// .NET Core async functions return Task subclass instances that trigger result wrapping
12951316

@@ -1303,17 +1324,11 @@ internal override object MarshalToScript(object obj, HostItemFlags flags)
13031324
{
13041325
if (testObject.GetType().IsAssignableToGenericType(typeof(Task<>), out var typeArgs))
13051326
{
1306-
using (Scope.Create(() => MiscHelpers.Exchange(ref bypassTaskPromiseConversion, true), oldValue => bypassTaskPromiseConversion = oldValue))
1307-
{
1308-
obj = typeof(TaskConverter<>).MakeSpecificType(typeArgs).InvokeMember("ToPromise", BindingFlags.InvokeMethod | BindingFlags.Public | BindingFlags.Static, null, null, new[] {testObject, this});
1309-
}
1327+
obj = typeof(TaskConverter<>).MakeSpecificType(typeArgs).InvokeMember("ToPromise", BindingFlags.InvokeMethod | BindingFlags.Public | BindingFlags.Static, null, null, new[] {testObject, this});
13101328
}
13111329
else if (testObject is Task task)
13121330
{
1313-
using (Scope.Create(() => MiscHelpers.Exchange(ref bypassTaskPromiseConversion, true), oldValue => bypassTaskPromiseConversion = oldValue))
1314-
{
1315-
obj = task.ToPromise(this);
1316-
}
1331+
obj = task.ToPromise(this);
13171332
}
13181333
}
13191334
}
@@ -1378,12 +1393,9 @@ internal override object MarshalToHost(object obj, bool preserveHostTarget)
13781393
}
13791394

13801395
var scriptItem = V8ScriptItem.Wrap(this, obj);
1381-
if (engineFlags.HasFlag(V8ScriptEngineFlags.EnableTaskPromiseConversion) && !bypassTaskPromiseConversion && (obj is IV8Object v8Object) && v8Object.IsPromise())
1396+
if (engineFlags.HasFlag(V8ScriptEngineFlags.EnableTaskPromiseConversion) && (obj is IV8Object v8Object) && v8Object.IsPromise())
13821397
{
1383-
using (Scope.Create(() => MiscHelpers.Exchange(ref bypassTaskPromiseConversion, true), oldValue => bypassTaskPromiseConversion = oldValue))
1384-
{
1385-
return scriptItem.ToTask();
1386-
}
1398+
return scriptItem.ToTask();
13871399
}
13881400

13891401
return scriptItem;
@@ -1505,20 +1517,51 @@ protected override void Dispose(bool disposing)
15051517

15061518
uint IJavaScriptEngine.BaseLanguageVersion => 8;
15071519

1508-
void IJavaScriptEngine.CompletePromiseWithResult<T>(Task<T> task, object resolve, object reject)
1520+
object IJavaScriptEngine.CreatePromiseForTask<T>(Task<T> task)
15091521
{
1510-
using (Scope.Create(() => MiscHelpers.Exchange(ref bypassTaskPromiseConversion, true), oldValue => bypassTaskPromiseConversion = oldValue))
1522+
return CreatePromise((resolve, reject) =>
15111523
{
1512-
Script.EngineInternal.completePromiseWithResult(task, resolve, reject);
1513-
}
1524+
task.ContinueWith(_ => CompletePromise(task, resolve, reject), TaskContinuationOptions.ExecuteSynchronously);
1525+
});
15141526
}
15151527

1516-
void IJavaScriptEngine.CompletePromise(Task task, object resolve, object reject)
1528+
object IJavaScriptEngine.CreatePromiseForTask(Task task)
15171529
{
1518-
using (Scope.Create(() => MiscHelpers.Exchange(ref bypassTaskPromiseConversion, true), oldValue => bypassTaskPromiseConversion = oldValue))
1530+
return CreatePromise((resolve, reject) =>
15191531
{
1520-
Script.EngineInternal.completePromise(task, resolve, reject);
1532+
task.ContinueWith(_ => CompletePromise(task, resolve, reject), TaskContinuationOptions.ExecuteSynchronously);
1533+
});
1534+
}
1535+
1536+
Task<object> IJavaScriptEngine.CreateTaskForPromise(ScriptObject promise)
1537+
{
1538+
if (!(promise is V8ScriptItem v8Promise) || !v8Promise.IsPromise())
1539+
{
1540+
throw new ArgumentException("The object is not a V8 promise", nameof(promise));
15211541
}
1542+
1543+
var source = new TaskCompletionSource<object>();
1544+
1545+
Action<object> onResolved = result =>
1546+
{
1547+
source.SetResult(result);
1548+
};
1549+
1550+
Action<object> onRejected = error =>
1551+
{
1552+
try
1553+
{
1554+
Script.EngineInternal.throwValue(error);
1555+
}
1556+
catch (Exception exception)
1557+
{
1558+
source.SetException(exception);
1559+
}
1560+
};
1561+
1562+
v8Promise.InvokeMethod(false, "then", onResolved, onRejected);
1563+
1564+
return source.Task;
15221565
}
15231566

15241567
#endregion

‎ClearScript/V8/V8ScriptItem.cs‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,24 @@ public static object Wrap(V8ScriptEngine engine, object obj)
7676
return obj;
7777
}
7878

79+
public bool IsPromise()
80+
{
81+
return target.IsPromise();
82+
}
83+
84+
public object InvokeMethod(bool marshalResult, string name, params object[] args)
85+
{
86+
VerifyNotDisposed();
87+
88+
var result = engine.ScriptInvoke(() => target.InvokeMethod(name, engine.MarshalToScript(args)));
89+
if (marshalResult)
90+
{
91+
return engine.MarshalToHost(result, false);
92+
}
93+
94+
return result;
95+
}
96+
7997
private void VerifyNotDisposed()
8098
{
8199
if (disposedFlag.IsSet)
@@ -255,8 +273,7 @@ public override object Invoke(bool asConstructor, params object[] args)
255273

256274
public override object InvokeMethod(string name, params object[] args)
257275
{
258-
VerifyNotDisposed();
259-
return engine.MarshalToHost(engine.ScriptInvoke(() => target.InvokeMethod(name, engine.MarshalToScript(args))), false);
276+
return InvokeMethod(true, name, args);
260277
}
261278

262279
#endregion

‎ClearScript/Windows/JScriptEngine.Unix.cs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,17 @@ protected JScriptEngine(string progID, string name, string fileNameExtensions, W
9090

9191
uint IJavaScriptEngine.BaseLanguageVersion => 3;
9292

93-
void IJavaScriptEngine.CompletePromiseWithResult<T>(Task<T> task, object resolve, object reject)
93+
object IJavaScriptEngine.CreatePromiseForTask<T>(Task<T> task)
9494
{
9595
throw new NotImplementedException();
9696
}
9797

98-
void IJavaScriptEngine.CompletePromise(Task task, object resolve, object reject)
98+
object IJavaScriptEngine.CreatePromiseForTask(Task task)
99+
{
100+
throw new NotImplementedException();
101+
}
102+
103+
Task<object> IJavaScriptEngine.CreateTaskForPromise(ScriptObject promise)
99104
{
100105
throw new NotImplementedException();
101106
}

‎ClearScript/Windows/JScriptEngine.cs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,12 +299,17 @@ internal override object Execute(UniqueDocumentInfo documentInfo, string code, b
299299

300300
uint IJavaScriptEngine.BaseLanguageVersion => 3;
301301

302-
void IJavaScriptEngine.CompletePromiseWithResult<T>(Task<T> task, object resolve, object reject)
302+
object IJavaScriptEngine.CreatePromiseForTask<T>(Task<T> task)
303303
{
304304
throw new NotImplementedException();
305305
}
306306

307-
void IJavaScriptEngine.CompletePromise(Task task, object resolve, object reject)
307+
object IJavaScriptEngine.CreatePromiseForTask(Task task)
308+
{
309+
throw new NotImplementedException();
310+
}
311+
312+
Task<object> IJavaScriptEngine.CreateTaskForPromise(ScriptObject promise)
308313
{
309314
throw new NotImplementedException();
310315
}

‎ClearScriptBenchmarks/Properties/AssemblyInfo.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,4 @@
1313
[assembly: ComVisible(false)]
1414
[assembly: AssemblyVersion("7.0.0")]
1515
[assembly: AssemblyFileVersion("7.0.0")]
16-
[assembly: AssemblyInformationalVersion("7.0.0-rc2")]
16+
[assembly: AssemblyInformationalVersion("7.0.0-rc3")]

‎ClearScriptConsole/Properties/AssemblyInfo.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,4 @@
1313
[assembly: ComVisible(false)]
1414
[assembly: AssemblyVersion("7.0.0")]
1515
[assembly: AssemblyFileVersion("7.0.0")]
16-
[assembly: AssemblyInformationalVersion("7.0.0-rc2")]
16+
[assembly: AssemblyInformationalVersion("7.0.0-rc3")]

0 commit comments

Comments
 (0)