Skip to content

[browser] Align CLR completeTaskWrapper to exit on exception, matching Mono#127314

Merged
pavelsavara merged 3 commits intodotnet:mainfrom
pavelsavara:fix/align-completetaskwrapper-exit
Apr 24, 2026
Merged

[browser] Align CLR completeTaskWrapper to exit on exception, matching Mono#127314
pavelsavara merged 3 commits intodotnet:mainfrom
pavelsavara:fix/align-completetaskwrapper-exit

Conversation

@pavelsavara
Copy link
Copy Markdown
Member

@pavelsavara pavelsavara commented Apr 23, 2026

Fixes #125587

Summary

The CLR implementation of completeTaskWrapper silently swallowed exceptions during Promise-to-Task marshalling, while the Mono implementation called mono_exit(1, ex) to terminate the runtime. An exception here means the managed Task will never complete (hanging forever) and the GCHandle/proxy state is corrupted.

This PR aligns the CLR side by calling dotnetLoaderExports.exit(1, ex), which normalizes the error, notifies exit listeners, tears down timers, and then quits — matching the Mono behavior.

Changes

  • exchange.ts — Added exit to LoaderExports type and LoaderExportsTable
  • loader/index.ts — Added exit to the loader functions object and serialization table
  • cross-module/index.ts — Updated deserialization to read exit at the new table index
  • marshaled-types.ts — CLR completeTaskWrapper catch block now calls exit(1, ex) instead of silently swallowing

…o behavior

Fixes dotnet#125587

The CLR implementation of completeTaskWrapper silently swallowed exceptions
during Promise-to-Task marshalling, while the Mono implementation called
mono_exit(1, ex) to terminate the runtime. This aligns the CLR side by
calling dotnetLoaderExports.exit(1, ex), which normalizes the error,
notifies exit listeners, tears down timers, and then quits.

Also adds exit() to the LoaderExports cross-module interface so it can
be called from the interop module.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Aligns the CoreCLR browser runtime’s Promise→Task marshalling error path with Mono by exiting the runtime when completeTaskWrapper throws, preventing permanently-hung managed Tasks and corrupted interop state.

Changes:

  • Added exit to the loader exports shape (LoaderExports) and its cross-module exchange table.
  • Wired exit through loader serialization (loaderExportsToTable) and cross-module deserialization (loaderExportsFromTable).
  • Updated PromiseHolder.completeTaskWrapper to call dotnetLoaderExports.exit(1, ex) when marshalling throws.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshaled-types.ts Calls loader exit() when Promise→Task completion marshalling throws.
src/native/libs/Common/JavaScript/types/exchange.ts Extends LoaderExports/LoaderExportsTable to include exit.
src/native/libs/Common/JavaScript/loader/index.ts Adds exit to the loader exports object and serialization table.
src/native/libs/Common/JavaScript/cross-module/index.ts Updates deserialization indices to include exit from the table.
Comments suppressed due to low confidence (1)

src/native/libs/Common/JavaScript/loader/index.ts:123

  • loaderExportsToTable() currently inserts the new exit entry before normalizeException, which changes the numeric indices of all subsequent slots. Since this table is used as a cross-bundle ABI, new exports should be appended to the end to keep existing indices stable; move exit to the end of the returned array and adjust loaderExportsFromTable() accordingly.
            dotnetLoaderExports.addOnExitListener,
            dotnetLoaderExports.abortStartup,
            dotnetLoaderExports.quitNow,
            dotnetLoaderExports.exit,
            dotnetLoaderExports.normalizeException,
            dotnetLoaderExports.fetchSatelliteAssemblies,
            dotnetLoaderExports.fetchLazyAssembly,
        ];

Comment thread src/native/libs/Common/JavaScript/types/exchange.ts
Comment thread src/native/libs/Common/JavaScript/cross-module/index.ts
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 23, 2026 09:34
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread src/native/libs/Common/JavaScript/loader/index.ts
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Copy link
Copy Markdown
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 and my copilot 👍

@pavelsavara pavelsavara added the os-browser Browser variant of arch-wasm label Apr 24, 2026
@pavelsavara
Copy link
Copy Markdown
Member Author

/ba-g unrelated failures

@pavelsavara pavelsavara merged commit 16d111a into dotnet:main Apr 24, 2026
146 of 151 checks passed
@pavelsavara pavelsavara deleted the fix/align-completetaskwrapper-exit branch April 24, 2026 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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.

[browser] Mono/CLR deviate in implementation of exception handling during promise value marshalling

3 participants