-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[browser][wasm] Implement JavaScript managed object reference counting #37417
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- This still needs to have the code implement nullable
…http-interop # Conflicts: # src/libraries/System.Net.Http/src/System.Net.Http.csproj
- Throws `PlatformNotSupportedException` for properties and methods of the HttpMessageHandler abstract implementation.
- Addresses commit comments.
…http-interop
- For all of these vars, please replace them with the actual type, except when the type is obvious from a new or explicit cast on the right-hand side.
- Address review comments
Co-Authored-By: Marek Safar <[email protected]>
jkotas
reviewed
Jun 9, 2020
...m.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/AnyRef.cs
Outdated
Show resolved
Hide resolved
jkotas
reviewed
Jun 9, 2020
...m.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/AnyRef.cs
Outdated
Show resolved
Hide resolved
jkotas
reviewed
Jun 9, 2020
...m.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/AnyRef.cs
Show resolved
Hide resolved
jkotas
reviewed
Jun 9, 2020
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Outdated
Show resolved
Hide resolved
…reference-counting # Conflicts: # src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/AnyRef.cs # src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
marek-safar
requested changes
Jun 10, 2020
...m.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/AnyRef.cs
Outdated
Show resolved
Hide resolved
...m.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/AnyRef.cs
Outdated
Show resolved
Hide resolved
...m.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/AnyRef.cs
Outdated
Show resolved
Hide resolved
...m.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/AnyRef.cs
Outdated
Show resolved
Hide resolved
...m.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/AnyRef.cs
Outdated
Show resolved
Hide resolved
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Outdated
Show resolved
Hide resolved
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Outdated
Show resolved
Hide resolved
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Show resolved
Hide resolved
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Outdated
Show resolved
Hide resolved
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Outdated
Show resolved
Hide resolved
marek-safar
reviewed
Jun 10, 2020
...m.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/AnyRef.cs
Outdated
Show resolved
Hide resolved
...Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSObject.cs
Outdated
Show resolved
Hide resolved
...Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSObject.cs
Outdated
Show resolved
Hide resolved
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Outdated
Show resolved
Hide resolved
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Outdated
Show resolved
Hide resolved
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Outdated
Show resolved
Hide resolved
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Outdated
Show resolved
Hide resolved
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Outdated
Show resolved
Hide resolved
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Outdated
Show resolved
Hide resolved
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Outdated
Show resolved
Hide resolved
marek-safar
reviewed
Jun 11, 2020
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Outdated
Show resolved
Hide resolved
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Show resolved
Hide resolved
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Outdated
Show resolved
Hide resolved
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Show resolved
Hide resolved
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Outdated
Show resolved
Hide resolved
} | ||
else | ||
{ | ||
Console.WriteLine($"\tSafeHandleReleaseByHandle: did not find active target {jsId} / target: {reference.Target}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are these not asserts?
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Outdated
Show resolved
Hide resolved
marek-safar
approved these changes
Jun 11, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Wrap the javascript handle in a SafeHandleZeroOrMinusOneIsInvalid implementation.
AnyRef
AnyRef
to be abstract and deriving fromSafeHandleZeroOrMinusOneIsInvalid
ownsHandle
GCHandle.Alloc
creates a normal allocation or a weak allocation depending on the ownsHandle value passed.JSObject
ReleaseHandle
that is overridden fromAnyRef
Dispose
and Finalizers definitions from the objects. This will be taken care of byAnyRef
which derives fromSafeHandleZeroOrMinusOneIsInvalid
ownsHandle
Runtime
_boundObjects
dictionary that tracks all the objects that have been bound from JavaScript objects to managed code is now aWeakReference
.binding_support
lib.JSObject.
Add support for marshaling the
AnyRef
SafeHandle.JSObject
reference handle is incremented.JSObject
reference is decremented.GlobalObject
where the object is set withownsHandle
= false so that it is not released.Support freeing delegates marshaled to the bindings.
Every delegate that was passed to the bindings code was kept forever causing leaks.
Delegates are now not kept alive when GC'd.
Delegates using lambda expressions forced the developer to explicitly free the delegate.
FreeObject (foreachAction);
If a delegate is collected and the javascript code calls that delegate function an exception is thrown instead of inexplicably crashing with null exception error.