Skip to content

Commit 3a88b89

Browse files
pavelsavarapavelsavara
authored
[browser][mt] Release all proxies of C# and JS objects (#88052)
1 parent f21f2ff commit 3a88b89

File tree

33 files changed

+425
-114
lines changed

33 files changed

+425
-114
lines changed

‎src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,7 @@ private async Task CancelationHelper(Task jsTask, CancellationToken cancellation
601601
FastState = WebSocketState.Aborted;
602602
throw new OperationCanceledException(cancellationToken);
603603
}
604-
if (ex.Message == "OperationCanceledException")
604+
if (ex.Message == "Error: OperationCanceledException")
605605
{
606606
FastState = WebSocketState.Aborted;
607607
throw new OperationCanceledException("The operation was cancelled.", ex, cancellationToken);

‎src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Interop/JavaScriptExports.cs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -143,11 +143,8 @@ public static void ReleaseJSOwnedObjectByGCHandle(JSMarshalerArgument* arguments
143143
{
144144
GCHandle handle = (GCHandle)arg_1.slot.GCHandle;
145145

146-
lock (JSHostImplementation.s_gcHandleFromJSOwnedObject)
147-
{
148-
JSHostImplementation.s_gcHandleFromJSOwnedObject.Remove(handle.Target!);
149-
handle.Free();
150-
}
146+
JSHostImplementation.ThreadJsOwnedObjects.Remove(handle.Target!);
147+
handle.Free();
151148
}
152149
catch (Exception ex)
153150
{

‎src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSFunctionBinding.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ public static JSFunctionBinding BindManagedFunction(string fullyQualifiedName, i
172172
[MethodImpl(MethodImplOptions.AggressiveInlining)]
173173
internal static unsafe void InvokeJSImpl(JSObject jsFunction, Span<JSMarshalerArgument> arguments)
174174
{
175+
ObjectDisposedException.ThrowIf(jsFunction.IsDisposed, jsFunction);
175176
#if FEATURE_WASM_THREADS
176177
JSObject.AssertThreadAffinity(jsFunction);
177178
#endif
@@ -205,10 +206,7 @@ internal static unsafe void InvokeImportImpl(IntPtr fnHandle, Span<JSMarshalerAr
205206
internal static unsafe JSFunctionBinding BindJSFunctionImpl(string functionName, string moduleName, ReadOnlySpan<JSMarshalerType> signatures)
206207
{
207208
#if FEATURE_WASM_THREADS
208-
if (JSSynchronizationContext.CurrentJSSynchronizationContext == null)
209-
{
210-
throw new InvalidOperationException("Please use dedicated worker for working with JavaScript interop. See https://github.com/dotnet/runtime/blob/main/src/mono/wasm/threads.md#JS-interop-on-dedicated-threads ");
211-
}
209+
JSSynchronizationContext.AssertWebWorkerContext();
212210
#endif
213211

214212
var signature = JSHostImplementation.GetMethodSignature(signatures);

‎src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSHost.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ public static JSObject GlobalThis
2121
{
2222
get
2323
{
24+
#if FEATURE_WASM_THREADS
25+
JSSynchronizationContext.AssertWebWorkerContext();
26+
#endif
2427
return JavaScriptImports.GetGlobalThis();
2528
}
2629
}
@@ -32,6 +35,9 @@ public static JSObject DotnetInstance
3235
{
3336
get
3437
{
38+
#if FEATURE_WASM_THREADS
39+
JSSynchronizationContext.AssertWebWorkerContext();
40+
#endif
3541
return JavaScriptImports.GetDotnetInstance();
3642
}
3743
}
@@ -47,6 +53,9 @@ public static JSObject DotnetInstance
4753
[MethodImpl(MethodImplOptions.AggressiveInlining)]
4854
public static Task<JSObject> ImportAsync(string moduleName, string moduleUrl, CancellationToken cancellationToken = default)
4955
{
56+
#if FEATURE_WASM_THREADS
57+
JSSynchronizationContext.AssertWebWorkerContext();
58+
#endif
5059
return JSHostImplementation.ImportAsync(moduleName, moduleUrl, cancellationToken);
5160
}
5261

‎src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSHostImplementation.cs

Lines changed: 73 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,28 @@ public static Dictionary<int, WeakReference<JSObject>> ThreadCsOwnedObjects
3232
}
3333

3434
// we use this to maintain identity of GCHandle for a managed object
35-
public static Dictionary<object, IntPtr> s_gcHandleFromJSOwnedObject = new Dictionary<object, IntPtr>(ReferenceEqualityComparer.Instance);
35+
#if FEATURE_WASM_THREADS
36+
[ThreadStatic]
37+
#endif
38+
private static Dictionary<object, IntPtr>? s_jsOwnedObjects;
39+
40+
public static Dictionary<object, IntPtr> ThreadJsOwnedObjects
41+
{
42+
get
43+
{
44+
s_jsOwnedObjects ??= new Dictionary<object, IntPtr>(ReferenceEqualityComparer.Instance);
45+
return s_jsOwnedObjects;
46+
}
47+
}
3648

3749
[MethodImpl(MethodImplOptions.AggressiveInlining)]
3850
public static void ReleaseCSOwnedObject(nint jsHandle)
3951
{
4052
if (jsHandle != IntPtr.Zero)
4153
{
54+
#if FEATURE_WASM_THREADS
55+
JSSynchronizationContext.AssertWebWorkerContext();
56+
#endif
4257
ThreadCsOwnedObjects.Remove((int)jsHandle);
4358
Interop.Runtime.ReleaseCSOwnedObject(jsHandle);
4459
}
@@ -64,19 +79,19 @@ public static void ReleaseCSOwnedObject(nint jsHandle)
6479
public static IntPtr GetJSOwnedObjectGCHandle(object obj, GCHandleType handleType = GCHandleType.Normal)
6580
{
6681
if (obj == null)
82+
{
6783
return IntPtr.Zero;
84+
}
6885

69-
IntPtr result;
70-
lock (s_gcHandleFromJSOwnedObject)
86+
IntPtr gcHandle;
87+
if (ThreadJsOwnedObjects.TryGetValue(obj, out gcHandle))
7188
{
72-
IntPtr gcHandle;
73-
if (s_gcHandleFromJSOwnedObject.TryGetValue(obj, out gcHandle))
74-
return gcHandle;
75-
76-
result = (IntPtr)GCHandle.Alloc(obj, handleType);
77-
s_gcHandleFromJSOwnedObject[obj] = result;
78-
return result;
89+
return gcHandle;
7990
}
91+
92+
IntPtr result = (IntPtr)GCHandle.Alloc(obj, handleType);
93+
ThreadJsOwnedObjects[obj] = result;
94+
return result;
8095
}
8196

8297
[MethodImpl(MethodImplOptions.AggressiveInlining)]
@@ -188,6 +203,9 @@ public static unsafe void FreeMethodSignatureBuffer(JSFunctionBinding signature)
188203

189204
public static JSObject CreateCSOwnedProxy(nint jsHandle)
190205
{
206+
#if FEATURE_WASM_THREADS
207+
JSSynchronizationContext.AssertWebWorkerContext();
208+
#endif
191209
JSObject? res;
192210

193211
if (!ThreadCsOwnedObjects.TryGetValue((int)jsHandle, out WeakReference<JSObject>? reference) ||
@@ -244,14 +262,55 @@ public static void InstallWebWorkerInterop(bool installJSSynchronizationContext,
244262

245263
public static void UninstallWebWorkerInterop()
246264
{
247-
var ctx = SynchronizationContext.Current as JSSynchronizationContext;
265+
var ctx = JSSynchronizationContext.CurrentJSSynchronizationContext;
248266
var uninstallJSSynchronizationContext = ctx != null;
249267
if (uninstallJSSynchronizationContext)
250268
{
251-
SynchronizationContext.SetSynchronizationContext(ctx!.previousSynchronizationContext);
252-
JSSynchronizationContext.CurrentJSSynchronizationContext = null;
253-
ctx.isDisposed = true;
269+
try
270+
{
271+
foreach (var jsObjectWeak in ThreadCsOwnedObjects.Values)
272+
{
273+
if (jsObjectWeak.TryGetTarget(out var jso))
274+
{
275+
jso.Dispose();
276+
}
277+
}
278+
foreach (var gch in ThreadJsOwnedObjects.Values)
279+
{
280+
GCHandle gcHandle = (GCHandle)gch;
281+
282+
// if this is pending promise we reject it
283+
if (gcHandle.Target is TaskCallback holder)
284+
{
285+
unsafe
286+
{
287+
holder.Callback!.Invoke(null);
288+
}
289+
}
290+
gcHandle.Free();
291+
}
292+
SynchronizationContext.SetSynchronizationContext(ctx!.previousSynchronizationContext);
293+
JSSynchronizationContext.CurrentJSSynchronizationContext = null;
294+
ctx.isDisposed = true;
295+
}
296+
catch(Exception ex)
297+
{
298+
Environment.FailFast($"Unexpected error in UninstallWebWorkerInterop, ManagedThreadId: {Thread.CurrentThread.ManagedThreadId}. " + ex);
299+
}
300+
}
301+
else
302+
{
303+
if (ThreadCsOwnedObjects.Count > 0)
304+
{
305+
Environment.FailFast($"There should be no JSObjects proxies on this thread, ManagedThreadId: {Thread.CurrentThread.ManagedThreadId}");
306+
}
307+
if (ThreadJsOwnedObjects.Count > 0)
308+
{
309+
Environment.FailFast($"There should be no JS proxies of managed objects on this thread, ManagedThreadId: {Thread.CurrentThread.ManagedThreadId}");
310+
}
254311
}
312+
ThreadCsOwnedObjects.Clear();
313+
ThreadJsOwnedObjects.Clear();
255314
Interop.Runtime.UninstallWebWorkerInterop(uninstallJSSynchronizationContext);
256315
}
257316

‎src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSObject.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ public partial class JSObject : IDisposable
2727
public bool HasProperty(string propertyName)
2828
{
2929
ObjectDisposedException.ThrowIf(IsDisposed, this);
30+
#if FEATURE_WASM_THREADS
31+
JSObject.AssertThreadAffinity(this);
32+
#endif
3033
return JavaScriptImports.HasProperty(this, propertyName);
3134
}
3235

‎src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSSynchronizationContext.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,16 @@ internal JSSynchronizationContext(Thread targetThread, IntPtr targetThreadId)
5555
{
5656
}
5757

58+
internal static void AssertWebWorkerContext()
59+
{
60+
#if FEATURE_WASM_THREADS
61+
if (CurrentJSSynchronizationContext == null)
62+
{
63+
throw new InvalidOperationException("Please use dedicated worker for working with JavaScript interop. See https://aka.ms/dotnet-JS-interop-threads");
64+
}
65+
#endif
66+
}
67+
5868
private JSSynchronizationContext(Thread targetThread, IntPtr targetThreadId, WorkItemQueueType queue)
5969
{
6070
TargetThread = targetThread;

‎src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Marshaling/JSMarshalerArgument.Task.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ public unsafe void ToManaged(out Task? value)
4949
TaskCompletionSource tcs = new TaskCompletionSource(holder);
5050
JSHostImplementation.ToManagedCallback callback = (JSMarshalerArgument* arguments_buffer) =>
5151
{
52+
if (arguments_buffer == null)
53+
{
54+
tcs.TrySetException(new TaskCanceledException("WebWorker which is origin of the Promise is being terminated."));
55+
return;
56+
}
5257
ref JSMarshalerArgument arg_2 = ref arguments_buffer[3]; // set by caller when this is SetException call
5358
// arg_3 set by caller when this is SetResult call, un-used here
5459
if (arg_2.slot.Type != MarshalerType.None)
@@ -88,6 +93,12 @@ public unsafe void ToManaged<T>(out Task<T>? value, ArgumentToManagedCallback<T>
8893
TaskCompletionSource<T> tcs = new TaskCompletionSource<T>(holder);
8994
JSHostImplementation.ToManagedCallback callback = (JSMarshalerArgument* arguments_buffer) =>
9095
{
96+
if (arguments_buffer == null)
97+
{
98+
tcs.TrySetException(new TaskCanceledException("WebWorker which is origin of the Promise is being terminated."));
99+
return;
100+
}
101+
91102
ref JSMarshalerArgument arg_2 = ref arguments_buffer[3]; // set by caller when this is SetException call
92103
ref JSMarshalerArgument arg_3 = ref arguments_buffer[4]; // set by caller when this is SetResult call
93104
if (arg_2.slot.Type != MarshalerType.None)

‎src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/SynchronizationContextExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
namespace System.Runtime.InteropServices.JavaScript
88
{
99
/// <summary>
10-
/// This is draft for possible public API of SynchronizationContext
10+
/// Extensions of SynchronizationContext which propagate errors and return values
1111
/// </summary>
1212
public static class SynchronizationContextExtension
1313
{

0 commit comments

Comments
 (0)