[wasm] Redesign of JS objects marshaling and lifecycle #57098

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.

Sign up for GitHub

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

Jump to bottom

Merged

lewing merged 33 commits into dotnet:main from pavelsavara:wasm_interop_cs_owned

Aug 17, 2021

Member

pavelsavara commented Aug 9, 2021 •

edited

Loading

switched from gc_handle to js_handle for all JSObject marshaling
got rid of AnyRefHandle
got rid of bool ownsHandle
got rid of mono_wasm_owned_objects_frames and mono_wasm_owned_objects_LMF
got rid of ref counting code on AnyRef
got rid of AnyRef and HostObjectBase types
simplified instantiation of JSObject to single call
improves the way how in-flight references are added
asserts for disposed JSObject for fast fail
implements fallback strategy when old Safari doesn't have FinalizationRegistry and WeakRef

Fixes #56303

pavelsavara added arch-wasm area-System.Runtime.InteropServices.JavaScript labels

pavelsavara requested review from kg and lewing

August 9, 2021 22:30

ghost commented Aug 9, 2021

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

Issue Details

null

Author:	pavelsavara
Assignees:	-
Labels:	`arch-wasm`, `area-System.Runtime.InteropServices.JavaScript`
Milestone:	-

kg reviewed

View reviewed changes

...e.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/AnyRef.cs Outdated Show resolved Hide resolved

kg reviewed

View reviewed changes

...Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Function.cs Outdated Show resolved Hide resolved

kg reviewed

View reviewed changes

....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs Outdated Show resolved Hide resolved

kg reviewed

View reviewed changes

....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs Outdated Show resolved Hide resolved

kg reviewed

View reviewed changes

....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs Outdated Show resolved Hide resolved

kg reviewed

View reviewed changes

....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs Outdated Show resolved Hide resolved

kg reviewed

View reviewed changes

....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs Outdated Show resolved Hide resolved

kg reviewed

View reviewed changes

...ntime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/TypedArray.cs Outdated Show resolved Hide resolved

kg reviewed

View reviewed changes

src/mono/wasm/runtime/binding_support.js Outdated Show resolved Hide resolved

pavelsavara marked this pull request as ready for review

August 10, 2021 14:28

pavelsavara requested a review from marek-safar as a code owner

August 10, 2021 14:28

pavelsavara changed the title ~~[wasm][interop] Redesign of all marshaled JS objects~~ [wasm] Redesign of JS objects marshaling and lifecycle

karelz mentioned this pull request

Test failure: System.Net.Quic.Tests.QuicStreamTests_MsQuicProvider.BasicTest #56269

Closed

Member

lewing commented Aug 16, 2021

/azp run runtime

azure-pipelines bot commented Aug 16, 2021

Azure Pipelines successfully started running 1 pipeline(s).

pavelsavara requested a review from kg

August 16, 2021 14:22

kg requested changes

View reviewed changes

Member

kg left a comment

Latest changes look good aside from the comments I added in here and sent via convo

...InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.CS.Owned.cs Outdated Show resolved Hide resolved

src/mono/wasm/runtime/binding_support.js Outdated Show resolved Hide resolved

src/mono/wasm/runtime/binding_support.js Outdated Show resolved Hide resolved

src/mono/wasm/runtime/binding_support.js Outdated Show resolved Hide resolved

pavelsavara requested a review from kg

August 16, 2021 16:32

kg approved these changes

View reviewed changes

src/mono/wasm/runtime/binding_support.js Outdated Show resolved Hide resolved

pavelsavara added 6 commits

August 16, 2021 21:19


          thenable and datetime cleanup

8ce637c


          removed ownsHandle for readability, removed AnyRef from hierarchy, re…

d8326dd

…moved old refcount debug code


          rename _csOwnedObjects

4b96785


          rename _csOwnedObjects

0a4c611


          redirect to Inflight counter

99b05bb


          renames

0d218f9

pavelsavara added 21 commits

August 16, 2021 21:19


          feedback

53e2eb9


          added MappedType enum

36e864a


          naming consistency

e0b3b53


          feedback

7c743b1


          rename _cs_owned_objects_by_js_handle

efcd1a9


          split and rename files

a9212b7


          shouldAddInflight to more methods

4f24a9d


          improved in-flight references

c5f22be


          assert for JSObject not disposed

7d23f76


          introduce in-flight assert

cce3ccc


          move should_add_in_flight arg to fisrt position, so that we could bin…

e29416a

…d it in convert

doc

e4dd539


          cleanup

doc

d7345bc


          implement fallback for missing support of FInalizationRegistry/WeakRe…

6ee696a

…f in onlder browsers


          feedback

92b9a3b


          no polyfill feedback

685d2a5


          feedback

a6f7eda


          name consistency feedback

4a388ed


          use symbols for helper fields

f5cede0


          feedback

b8ad165

pavelsavara force-pushed the wasm_interop_cs_owned branch from 743abcb to b8ad165 Compare

August 16, 2021 19:19

Member Author

pavelsavara commented Aug 16, 2021

The failures are unrelated VB overflow issue and unrelated JIT issues on Android.
There is probably timing issue in System.Net.WebSockets.Client.Tests.CancelTest.ConnectAsync_Cancel_ThrowsCancellationException of staging lane. I don't know if it could be related or not.

Member

lewing commented Aug 17, 2021

browser failures are #57501

Member

lewing commented Aug 17, 2021

/azp run runtime

azure-pipelines bot commented Aug 17, 2021

Azure Pipelines successfully started running 1 pipeline(s).

lewing merged commit ead035b into dotnet:main

jaredpar mentioned this pull request

Inability to unzip assets during build on Unix x64 #32805

Closed

ghost locked as resolved and limited conversation to collaborators

pavelsavara deleted the wasm_interop_cs_owned branch

November 20, 2021 19:36

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasm area-System.Runtime.InteropServices.JavaScript