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

[wasm] pass ExitStatus when calling quit_ #64734

Merged
merged 8 commits into from
Feb 4, 2022

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Feb 3, 2022

For explanation Posix exit is implemented in emscripten as

  1. On NodeJS it would also call process.exit and stop immediately.
  2. Throwing javascript ExitStatus exception. That exception need to bubble thru all the call stack out into uncaughtException handler, which will recognize the class and swallow it. I think that going thru all the catch/finally statements it why it's designed this way. Also onV8 it's probably the only way how to stop, but I'm not sure now.

There is also noExitRuntime setting which could prevent that. We have it true on browser and I just set it to false on node and v8 in this PR.

Fixes #64727

@pavelsavara pavelsavara added this to the 7.0.0 milestone Feb 3, 2022
@ghost ghost assigned pavelsavara Feb 3, 2022
@ghost
Copy link

ghost commented Feb 3, 2022

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

Issue Details

Fixes #64727

Author: pavelsavara
Assignees: -
Labels:

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

Milestone: 7.0.0

fix logging
@pavelsavara
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara pavelsavara marked this pull request as ready for review February 3, 2022 10:28
@pavelsavara
Copy link
Member Author

Log

[wasm test] [11:29:54] fail: {"name":"ExitStatus","message":"Program terminated with exit(42)","status":42}
[wasm test]                  
[wasm test] [11:29:54] info: Process node exited with 1
[wasm test]                  
[wasm test] [11:29:54] fail: Application has finished with exit code 1 but 42 was expected
[wasm test] [11:29:54] dbug: Saving diagnostics data to '/datadisks/disk1/work/A4E10928/w/B36509BA/e/diagnostics.json'
[wasm test] XHarness exit code: 71 (GENERAL_FAILURE)

@radekdoulik
Copy link
Member

radekdoulik commented Feb 3, 2022

There's also:

[wasm test] [11:38:10] fail: Error
[wasm test]                  
[wasm test] [11:38:10] fail:     at Object.onAbort (test-main.js:157:25)
[wasm test]                  
[wasm test] [11:38:10] fail:     at abort (./dotnet.js:1973:24)
[wasm test]                  
[wasm test] [11:38:10] fail:     at Object.get (./dotnet.js:1116:7)
[wasm test]                  
[wasm test] [11:38:10] fail:     at Object.initializeImportsAndExports [as __initializeImportsAndExports] (./dotnet.js:3:83482)
[wasm test]                  
[wasm test] [11:38:10] fail:     at ./dotnet.js:7530:47
[wasm test]                  
[wasm test] [11:38:10] fail:     at test-main.js:123:12
[wasm test]                  
[wasm test] [11:38:10] fail: "Module.noExitRuntime has been replaced with plain noExitRuntime (the initial value can be provided on Module, but after startup the value is only looked for on a local variable of that name)"

in this log.

@radical
Copy link
Member

radical commented Feb 3, 2022

In case it helps - I had found that set_exit_code was getting called twice for node.

  1. set_exit_code(result), which would eventually call mono_wasm_exit-> exit, and that would throw an Error,
  2. causing set_exit_code(1, error) to get called then

As a fix I changed test-main.js to explicitly call process.exit(result) on node.

@pavelsavara
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara pavelsavara merged commit a74d7d1 into dotnet:main Feb 4, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Mar 6, 2022
@pavelsavara pavelsavara deleted the wasm_quit_fix branch July 14, 2022 20:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[wasm] NodeJS samples failing at exit with emsdk 2.0.34
3 participants