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.

Sign up for GitHub

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

Jump to bottom

[browser] improve promise resolve marshaling after exit #94288

Merged
merged 2 commits into from
Nov 2, 2023
Merged

[browser] improve promise resolve marshaling after exit #94288

merged 2 commits into from
Nov 2, 2023

Conversation

pavelsavara
Copy link
Member

Scenario:

  • after WS or HTTP unit tests we call forceDisposeProxies
  • that will abort pending WebSockets and reject pending promises
  • some of the pending promises only propagate to the marshaling to C# only after next tick of the browser loop
  • at which point, the emscripten/mono is already exited and can't marshal rejections
    • also, those handles are already disposed

Solution, don't try to marshal promise resolutions after runtime exit.

Related issue #89425

@pavelsavara pavelsavara added arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm labels Nov 2, 2023
@pavelsavara pavelsavara added this to the 9.0.0 milestone Nov 2, 2023
@pavelsavara pavelsavara self-assigned this Nov 2, 2023
@ghost
Copy link

ghost commented Nov 2, 2023

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

Issue Details

Scenario:

  • after WS or HTTP unit tests we call forceDisposeProxies
  • that will abort pending WebSockets and reject pending promises
  • some of the pending promises only propagate to the marshaling to C# only after next tick of the browser loop
  • at which point, the emscripten/mono is already exited and can't marshal rejections
    • also, those handles are already disposed

Solution, don't try to marshal promise resolutions after runtime exit.

Related issue #89425

Author: pavelsavara
Assignees: pavelsavara
Labels:

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

@pavelsavara pavelsavara marked this pull request as ready for review November 2, 2023 10:34
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@build-analysis build-analysis bot mentioned this pull request Nov 2, 2023
Closed
This was referenced Nov 2, 2023
Closed
Open
Open
@pavelsavara pavelsavara merged commit 43d164d into dotnet:main Nov 2, 2023
34 checks passed
@pavelsavara pavelsavara deleted the browser_ws_dispose branch November 2, 2023 16:48
@ghost ghost locked as resolved and limited conversation to collaborators Dec 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@maraf maraf maraf approved these changes

@lewing lewing Awaiting requested review from lewing lewing is a code owner

@kg kg Awaiting requested review from kg

Assignees

@pavelsavara pavelsavara

Labels
arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm
Projects
None yet
Milestone
9.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@pavelsavara @maraf