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] mono_exit improvements #88387

Merged
merged 28 commits into from
Jul 15, 2023
Merged

[browser] mono_exit improvements #88387

merged 28 commits into from
Jul 15, 2023

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Jul 4, 2023 •
edited
Loading

  • call mono_wasm_exit from mono_exit
    • this will tell mono that shutting_down_inited and then join all mono threads
    • after that emscripten will also immediately terminate all WebWorkers
  • add assert_runtime_running to many entry points
  • handle onExit
  • added withAssertAfterExit and assertAfterExit which is default true for shell/node
  • changed mono_assert to abort the program and introduced mono_check alternative which just throws exception.
  • took over browser-shutdown sample from @kg's PR
  • replace abort_startup with mono_exit

Alternative to #87043

@pavelsavara pavelsavara added arch-wasm WebAssembly architecture os-browser Browser variant of arch-wasm labels Jul 4, 2023
@pavelsavara pavelsavara added this to the 8.0.0 milestone Jul 4, 2023
@pavelsavara pavelsavara self-assigned this Jul 4, 2023
@ghost
Copy link

ghost commented Jul 4, 2023

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

Issue Details
  • assert_runtime_running
  • handle onExit
  • call mono_wasm_exit from mono_exit
Author: pavelsavara
Assignees: pavelsavara
Labels:

arch-wasm, os-browser
Milestone: 8.0.0

@pavelsavara pavelsavara mentioned this pull request Jul 4, 2023
Merged
@pavelsavara pavelsavara marked this pull request as ready for review July 4, 2023 20:28
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

kg
kg reviewed Jul 4, 2023
View reviewed changes
src/mono/wasm/runtime/marshal.ts Outdated Show resolved Hide resolved
kg
kg approved these changes Jul 4, 2023
View reviewed changes
Copy link
Member

@kg kg left a comment

Choose a reason for hiding this comment

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

Looks fine other than comments

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jul 7, 2023
Closed
Closed
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jul 13, 2023
Open
Closed
Closed
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@build-analysis build-analysis bot mentioned this pull request Jul 14, 2023
Closed
@pavelsavara
Merge branch 'main' into browser_exit
c5e0c48 
# Conflicts:
#	src/mono/wasm/runtime/http.ts
#	src/mono/wasm/runtime/web-socket.ts
@pavelsavara
fixx merge
66bfa87
@pavelsavara
fix abort + sample
5c0faf0
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

No commit pushedDate could be found for PR 88387 in repo dotnet/runtime

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@runfoapp runfoapp bot mentioned this pull request Jul 14, 2023
Open
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

No commit pushedDate could be found for PR 88387 in repo dotnet/runtime

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@build-analysis build-analysis bot mentioned this pull request Jul 15, 2023
Closed
@pavelsavara pavelsavara merged commit 7980421 into dotnet:main Jul 15, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 14, 2023
@pavelsavara pavelsavara deleted the browser_exit branch September 2, 2024 15:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lambdageek lambdageek lambdageek left review comments

@kg kg kg approved these changes

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

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

@radical radical Awaiting requested review from radical

Assignees

@pavelsavara pavelsavara

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

Successfully merging this pull request may close these issues.
4 participants
@pavelsavara @kg @lambdageek @lewing