Skip to content

Migrate various parts of the bindings code to use roots - #55530

Merged
kg merged 8 commits into
dotnet:mainfrom
kg:rootify-bindings
Jul 14, 2021
Merged

Migrate various parts of the bindings code to use roots#55530
kg merged 8 commits into
dotnet:mainfrom
kg:rootify-bindings

Conversation

@kg

@kgkg commented Jul 12, 2021

Copy link
Copy Markdown
Contributor

We have a lot of code that passes raw pointers to managed objects around when they should really be using roots instead (i.e. root.value) so that if the GC relocates the object, they won't be using the old address. This PR migrates a bunch of that code so that it uses root objects.

A lot of code remains that does use raw objects, but I read over all of it and the cases appear to be safe because they immediately return it (or return it without calling any other functions).

@kg
kg requested a review from marek-safar as a code ownerJuly 12, 2021 19:13
@kgkg added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) and removed area-System.Runtime.InteropServices.JavaScript labels Jul 12, 2021
@kg
kg requested a review from lewingJuly 12, 2021 19:13
Comment threadsrc/mono/wasm/runtime/binding_support.js
Comment threadsrc/mono/wasm/runtime/binding_support.js Outdated
@lewing

Copy link
Copy Markdown
Member

failure is #55588

@kg
kg merged commit 8d365a0 into dotnet:mainJul 14, 2021
@ghostghost locked as resolved and limited conversation to collaborators Aug 13, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Runtime.InteropServices.JavaScriptNO-MERGEThe PR is not ready for merge yet (see discussion for detailed reasons)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@kg@lewing@pavelsavara