Skip to content

[wasm] Add an internal GC lock API #82646

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

Merged
merged 5 commits into from
Mar 1, 2023

Conversation

lambdageek
Copy link
Member

No description provided.

@ghost ghost assigned lambdageek Feb 24, 2023
@lambdageek
Copy link
Member Author

/cc @pavelsavara @lewing

@lambdageek lambdageek added arch-wasm WebAssembly architecture area-GC-mono labels Feb 24, 2023
@ghost
Copy link

ghost commented Feb 24, 2023

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

Issue Details

null

Author: lambdageek
Assignees: lambdageek
Labels:

arch-wasm, area-GC-mono

Milestone: -

@lambdageek lambdageek force-pushed the feature-blazor-gc-lock branch from ab2380a to dc936c1 Compare February 25, 2023 03:20
This is an exported API, so we're normally in GC Safe mode on entry
@lambdageek lambdageek changed the title [wasm] Add a GC lock API and deprecate it [wasm] Add an internal GC lock API Feb 27, 2023
@lambdageek
Copy link
Member Author

Proposed Blazor change is dotnet/aspnetcore#46909

Copy link
Member

@pavelsavara pavelsavara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ts changes LGTM

@lambdageek lambdageek marked this pull request as ready for review February 27, 2023 18:17
@naricc
Copy link
Contributor

naricc commented Feb 27, 2023

Changes look good to me, I was just wondering what the use case is; would it make more sense to implement GC.TryStartNoGCRegion instead?

@lambdageek
Copy link
Member Author

Changes look good to me, I was just wondering what the use case is; would it make more sense to implement GC.TryStartNoGCRegion instead?

Not sure a C# API is what we want in this case. the Blazor rendering code is in JS. Although if we end up implementing that API, we could probably update these new APIs to use the same underlying mechanism.

@BrzVlad
Copy link
Member

BrzVlad commented Feb 27, 2023

Why does the blazor rendering code need to lock on gc ?

@lambdageek
Copy link
Member Author

Why does the blazor rendering code need to lock on gc ?

So the way blazor rendering works is that on the C# side they build up a managed object that describes the DOM changes that they want to apply, and then on the JS side they run something that crawls over this object and applies the DOM mutations. They use unsafe MonoObject* accesses (ie, they get field offsets and then load pointers) to traverse the render tree. The actual JS rendering step runs in a single browser event iteration and is purely JS code - it doesn't invoke any managed code.

In single-threaded wasm this is GC safe, because there's a single thread and it's currently busy doing JS work.
In multi-threaded wasm, there could be background tasks that might trigger a GC and cause the render tree to move.

In the short term it's not possible to change Blazor to use GC handles or anything like that. So our plan is:

  1. give them a way to globally block any GC
  2. rewrite their rendering pass to use a GC-safe API (for example something like Add new public APIs for safely manipulating managed pointers #69458)

@lambdageek
Copy link
Member Author

@BrzVlad any objections?

@lambdageek lambdageek merged commit 5b89020 into dotnet:main Mar 1, 2023
lambdageek added a commit to dotnet/aspnetcore that referenced this pull request Mar 27, 2023
Take the runtime's GC lock when creating a heap lock.

**Important Notes**

1. While the runtime GC lock is locked, calling any C# function or doing anything that may trigger managed allocation will block the runtime.
2. In a threaded build, only the main browser thread is allowed to call the `mono_wasm_gc_lock` function
3. The runtime lock is not recursive
4. Background threads initially continue running, so the managed object graph may change even while the GC lock is held (although objects may not move).  Background threads could mutate valuetype fields of reachable objects, even while the GC lock is held. However background threads will block as soon as they try to do any allocation. 
5. The main browser thread may assume that managed heap objects will not move while the lock is held.

Corresponding runtime change dotnet/runtime#82646
@ghost ghost locked as resolved and limited conversation to collaborators Mar 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-GC-mono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants