Skip to content
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][MT] Keep track of additional JS keepalive sources for worker threads #85052

Closed
lambdageek opened this issue Apr 19, 2023 · 3 comments · Fixed by #99836
Closed

[browser][MT] Keep track of additional JS keepalive sources for worker threads #85052

lambdageek opened this issue Apr 19, 2023 · 3 comments · Fixed by #99836
Assignees
Labels
arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript in-pr There is an active PR which will close this issue when it is merged os-browser Browser variant of arch-wasm
Milestone

Comments

@lambdageek
Copy link
Member

In #84492 we added WebWorkerEventLoop.HasUnsettledInteropPromises that will be used by #84494 to keep a threadpool worker thread alive in the JS event loop as long as some promise exists that is needed by a JSImported Task in C#.

There could be, however, other reasons why a threadpool worker thread may need to stay alive: if the JS code has used setTimeout with a callback that depends on a JS proxy of a C# function or object, for example. These additional scenarios can be somewhat covered by creating promises that the C# code could await, but we should also have more robust tracking in our interop layer.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Apr 19, 2023
@ghost
Copy link

ghost commented Apr 19, 2023

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

Issue Details

In #84492 we added WebWorkerEventLoop.HasUnsettledInteropPromises that will be used by #84494 to keep a threadpool worker thread alive in the JS event loop as long as some promise exists that is needed by a JSImported Task in C#.

There could be, however, other reasons why a threadpool worker thread may need to stay alive: if the JS code has used setTimeout with a callback that depends on a JS proxy of a C# function or object, for example. These additional scenarios can be somewhat covered by creating promises that the C# code could await, but we should also have more robust tracking in our interop layer.

Author: lambdageek
Assignees: -
Labels:

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

Milestone: -

lambdageek added a commit to lambdageek/runtime that referenced this issue Apr 19, 2023
@lambdageek
Copy link
Member Author

/cc @pavelsavara

lambdageek added a commit that referenced this issue Apr 20, 2023
This is part of #84489 - landing support for async JS interop on threadpool threads in multi-threaded WebAssembly.

Provides two pieces of functionality:

1. A keepalive token that can be used to prevent the current POSIX thread from terminating when it returns from its thread start function, or from an invocation from the JS event loop.  When the last keepalive token is destroyed (assuming Emscripten isn't keeping the thread alive for other reasons) it will terminate as if by calling `pthread_exit` and the webworker will be made available to other threads

2. A `HasUnsettledInteropPromises` property that peeks `_js_owned_object_table` to see if there are any promises created by the interop subsystem that have not been fulfilled or rejected yet.



* [mono] add internal WebWorkerEventLoop utility class

Provides two pieces of functionality:

1. A keepalive token that can be used to prevent the current POSIX
thread from terminating when it returns from its thread start
function, or from an invocation from the JS event loop.  When the last
keepalive token is destroyed (assuming Emscripten isn't keeping the
thread alive for other reasons) it will terminate as if by calling
`pthread_exit` and the webworker will be made available to other
threads

2. A `HasUnsettledInteropPromises` property that peeks
`_js_owned_object_table` to see if there are any promises created by
the interop subsystem that have not been fulfilled or rejected yet.

* Use a per-thread unsettled promise count

for mono_wasm_eventloop_has_unsettled_interop_promises we
can't use the _js_owned_object_table size because it contains
other interop objects, not just promises

* remove old emscripten keepalive workaround hack

* Add a more general getter for JS interop to keep a webworker alive

And link to #85052 for more details

* fixup docs
@lewing lewing added this to the 9.0.0 milestone Jul 21, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jul 21, 2023
@lewing lewing modified the milestones: 9.0.0, 8.0.0 Jul 21, 2023
@pavelsavara
Copy link
Member

I think we will solve this by not allowing JS interop from managed thread pool. Eventually by dispatching the JS calls to original web worker or UI thread. And possibly by explicit API for creating WebWorker which could have JS interop, such worker would have keep alive right by design.

@pavelsavara pavelsavara changed the title [wasm-mt] Keep track of additional JS keepalive sources for worker threads [browser][MT] Keep track of additional JS keepalive sources for worker threads Nov 9, 2023
@pavelsavara pavelsavara added the os-browser Browser variant of arch-wasm label Nov 9, 2023
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Mar 15, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Apr 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript in-pr There is an active PR which will close this issue when it is merged os-browser Browser variant of arch-wasm
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants