[wasm][MT] Incorrect handling of null target in ReleaseJSOwnedObjectByGCHandle

[kg]

Log

[19:47:52] info: [FAIL] System.Runtime.InteropServices.JavaScript.Tests.JSImportTest.JsImportArraySegmentOfInt32
[19:47:52] info: System.NullReferenceException : Object reference not set to an instance of an object.
[19:47:52] info:    at System.Runtime.InteropServices.JavaScript.JSMarshalerArgument.ToManaged(ArraySegment`1& )
[19:47:52] info:    at System.Runtime.InteropServices.JavaScript.Tests.JavaScriptTestHelper.echo1_ArraySegmentOfInt32(ArraySegment`1 value, Boolean edit)
[19:47:52] info:    at System.Runtime.InteropServices.JavaScript.Tests.JSImportTest.JsImportArraySegmentOfInt32()
[19:47:52] info:    at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
[19:47:52] info:    at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags )

runtime/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSProxyContext.cs

Lines 408 to 416 in 1c4755d

	GCHandle handle = (GCHandle)gcHandle;
	var target = handle.Target!;
	if (target is PromiseHolder holder2)
	{
	holder = holder2;
	}
	else
	{
	if (!ThreadJsOwnedObjects.Remove(target))

target can potentially be null, and it appears it was in this case.

[dotnet-policy-service]

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

[kg]

cc @pavelsavara
Can target ever be null in a single threaded scenario? Or MT only?

[pavelsavara]

Probably duplicate #104470
It seems to me that Why are we setting the target on an unoccupied slot? #104524 could be also the same thing

[pavelsavara]

Could this be the root cause ? #107974

[pavelsavara]

This is not Promise it's a ArraySegment<int> which we pin with GCHandle and create JS MemoryView with that gc_handle.
We also register that handle into thread local JSProxyContext.ThreadJsOwnedObjects for re-use.
The only place when we are doing handle.Free(); is in ReleaseJSOwnedObjectByGCHandle under lock which is only called when teardown_managed_proxy.

That is only happening when ArraySegment.dispose() which we don't do in this test.
It could also happen when _js_owned_object_finalized via FinalizationRegistry.

The fail happens on marshaling result of the JS call to echo1view() and maybe on MT, that could happen in parallel with FinalizationRegistry ?

[pavelsavara]

release_js_owned_object_by_gc_handle is invoke_async_jsexport(runtimeHelpers.ioThreadTID which is different thread than which is executing the unit test

[pavelsavara]

Unit test is synchronous call from deputy thread to UI thread via JSImport which will do mono_threads_wasm_sync_run_in_target_thread_vii and be blocked until mono_threads_wasm_sync_run_in_target_thread_done is called by JS.
The release_js_owned_object_by_gc_handle could be dispatched to IO thread and free the handle before the JSMarshalerArgument.ToManaged is done marshaling from that handle.