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] send cancel to abandoned threads with event loop during mono_exit #97441

Merged

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Jan 24, 2024

MT improvements

  • cancelThreads will exit all JSWebWorker which are still running
  • only register runtime into globalThis when it's ready started
    • register it also with correct ID on the webworker
    • this prevents workers to use instannce from dry run
  • don't load memory snapshot in worker
  • omit managed stack trace after runtime exited
  • improve logging prefix in MT, to contain thread type and time
  • move call mono_threads_wasm_on_thread_attached to better place and add parameters
  • simplified message passing between worker and main

testing

  • skip TimerTests on MT, where it doesn't make sense
  • fix WebWorkerTest
  • re-enable MT smoke tests in runtime pipeline
  • new -untilFailed xunit parameter for WasmTestRunner

…idle in the browser event loop) preventing to be joined during mono_exit

- new `-untilFailed` xunit parameter for WasmTestRunner
- cleanup CancellationToken in WebWorkerTest
@pavelsavara pavelsavara added arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm labels Jan 24, 2024
@pavelsavara pavelsavara added this to the 9.0.0 milestone Jan 24, 2024
@pavelsavara pavelsavara self-assigned this Jan 24, 2024
@ghost
Copy link

ghost commented Jan 24, 2024

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

Issue Details
  • cancelThreads will exit all JSWebWorker which are still running (idle in the browser event loop) preventing to be joined during mono_exit
  • new -untilFailed xunit parameter for WasmTestRunner
  • cleanup CancellationToken in WebWorkerTest

Solves #96628 (comment)

Author: pavelsavara
Assignees: pavelsavara
Labels:

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

Milestone: 9.0.0

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

 - register it also with correct ID on the webworker
 - don't load memory snapshot in worker
 - omit managed stack trace after runtime exited
 - fix WebWorkerTest
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@pavelsavara pavelsavara marked this pull request as ready for review January 25, 2024 08:39
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

pavelsavara commented Jan 27, 2024

I filled #97597 fixed

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@maraf maraf left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

src/mono/browser/runtime/interp-pgo.ts Outdated Show resolved Hide resolved
src/mono/browser/runtime/startup.ts Outdated Show resolved Hide resolved
pavelsavara and others added 2 commits January 29, 2024 12:01
Co-authored-by: Marek Fišera <mara@neptuo.com>
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

image
image

There are unrelated AOT failures

@pavelsavara pavelsavara merged commit fb953cc into dotnet:main Jan 29, 2024
157 of 165 checks passed
@pavelsavara pavelsavara deleted the browser_abandoned_jswebworker_timeout branch January 29, 2024 14:33
@github-actions github-actions bot locked and limited conversation to collaborators Feb 29, 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 os-browser Browser variant of arch-wasm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants