-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[browser] native - no sockets, no threads, no blocking #123962
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
base: main
Are you sure you want to change the base?
Conversation
|
Tagging subscribers to this area: @karelz, @dotnet/ncl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds support for the browser (WASM) target by providing stub implementations for features not available in the browser environment, specifically: no sockets, no threads, and no blocking operations.
Changes:
- Added browser-specific stub implementations for networking, interface addresses, network statistics, and console APIs
- Updated error messages in WASM-specific files from "WASI" to "WASM" for clarity
- Modified CoreCLR PAL to prevent blocking operations and thread suspension in single-threaded WASM
- Updated build configuration to disable socket support for browser targets and include browser-specific source files
- Added perftracing configuration for browser builds
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/libs/System.Native/pal_signal_wasm.c | Updated error messages from "WASI" to "WASM" |
| src/native/libs/System.Native/pal_networkstatistics_wasm.c | New file with stub implementations returning "not supported" for network statistics APIs |
| src/native/libs/System.Native/pal_networking_browser.c | New file with comprehensive stub implementations for all networking APIs, returning ENOTSUP |
| src/native/libs/System.Native/pal_io.c | Added early return for pipe() on WASM targets with ENOTSUP |
| src/native/libs/System.Native/pal_interfaceaddresses_browser.c | New file with stub implementations for interface enumeration APIs |
| src/native/libs/System.Native/pal_dynamicload_wasm.c | Updated error messages from "WASI" to "WASM" |
| src/native/libs/System.Native/pal_console_wasm.c | New console implementation for WASM with some working features (stdin/isatty) and stubs for unsupported features |
| src/native/libs/System.Native/CMakeLists.txt | Updated build configuration to differentiate between browser and WASI targets, using browser-specific source files |
| src/native/eventpipe/ds-ipc-pal-websocket.c | Added string duplication helper functions for eventpipe |
| src/mono/mono/profiler/log.c | Changed sys/socket.h include guard from HOST_WIN32 to HAVE_SYS_SOCKET_H |
| src/mono/mono/profiler/helper.c | Changed sys/socket.h include guard from HOST_WIN32 to HAVE_SYS_SOCKET_H |
| src/mono/mono/profiler/CMakeLists.txt | Disabled AOT profiler for browser targets |
| src/mono/mono/mini/cfgdump.c | Added guard for sys/socket.h include |
| src/mono/browser/runtime/dotnet.d.ts | Formatting fix: removed space before parentheses in function signatures |
| src/coreclr/pal/src/thread/threadsusp.cpp | Added guard to prevent thread suspension in single-threaded WASM |
| src/coreclr/pal/src/thread/thread.cpp | Added guard to prevent CREATE_SUSPENDED flag in single-threaded WASM |
| src/coreclr/pal/src/synchmgr/wait.cpp | Added guards to prevent blocking waits and made Sleep return immediately in WASM |
| src/coreclr/pal/src/synchmgr/synchmanager.cpp | Added assertion to prevent waiting on non-signaled objects in WASM |
| src/coreclr/pal/src/debug/debug.cpp | Added WASM-specific implementation of PAL_ProbeMemory using emscripten heap size |
| eng/native/tryrun.browser.cmake | Disabled HAVE_SYS_SOCKET_H for browser targets |
| eng/native.wasm.targets | Added perftracing configuration flags for browser builds |
| #if defined(__EMSCRIPTEN__) | ||
| if ((uintptr_t)((PBYTE)pBuffer + cbBuffer) <= emscripten_get_heap_size()) | ||
| { | ||
| return TRUE; | ||
| } | ||
| return FALSE; |
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fWriteAccess parameter is not used in the Emscripten implementation. This means the function cannot distinguish between read-only and writable memory validation. The function should either use this parameter to check write permissions (though this may be difficult in WASM), or at minimum document this limitation, or mark the parameter as unused with (void)fWriteAccess.
| assert(buffer != NULL || bufferSize == 0); | ||
| assert(bufferSize >= 0); | ||
|
|
||
| if (bufferSize < 0) |
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an extra leading space before the 'if' statement. This appears inconsistent with the indentation in the existing code. The line should start with 4 spaces for proper indentation alignment.
| if (bufferSize < 0) | |
| if (bufferSize < 0) |
|
|
||
| int32_t SystemNative_GetSignalForBreak(void) | ||
| { | ||
| return false; |
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function returns 'false' which is a boolean literal, but the return type is int32_t. This should return 0 instead of false for type consistency. The standard implementation in pal_console.c returns the value of g_signalForBreak (which is a bool but implicitly converted to int), while the WASI implementation also incorrectly returns 'false'. For clarity and correctness, this should return 0.
| return false; | |
| return 0; |
| int32_t SystemNative_InitializeTerminalAndSignalHandling(void) | ||
| { | ||
| return true; |
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function returns 'true' which is a boolean literal, but the return type is int32_t. This should return 1 instead of true for type consistency.
| #include <netinet/in.h> | ||
| #include <netdb.h> |
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file includes netinet/in.h and netdb.h without header guards, but the PR disables HAVE_SYS_SOCKET_H for browser in tryrun.browser.cmake. The file uses socket APIs (socket, connect, sockaddr_in, etc.) throughout and is conditionally compiled based on !HOST_WIN32. Since browser doesn't support sockets, this file will fail to compile for browser targets. The compile-time guard on line 11 should likely also exclude HOST_BROWSER, or these header includes need proper HAVE_* guards.
| #if defined(__EMSCRIPTEN__) | ||
| if ((uintptr_t)((PBYTE)pBuffer + cbBuffer) <= emscripten_get_heap_size()) |
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The addition (PBYTE)pBuffer + cbBuffer could potentially overflow if pBuffer is near the end of the address space. Consider checking for overflow before performing the comparison, for example by checking if cbBuffer > (SIZE_MAX - (uintptr_t)pBuffer) first.
| if (pThread->GetCreateSuspended()) | ||
| { | ||
| #ifdef __wasm__ | ||
| // CREATE_SUSPENDED is not supported in single-threaded WASM because it uses |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating threads is not supported in single threaded wasm. Should the scope of these ifdefs be much larger?
| goto TNW_exit; | ||
| } | ||
|
|
||
| #ifdef __wasm__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be FEATURE_SINGLE_THREADED?
| @@ -751,6 +755,13 @@ PAL_ProbeMemory( | |||
| DWORD cbBuffer, | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole method should be ifdefed out for Wasm.
|
Whole this PR makes emscripten emulator smaller only by 20KB of JavaScript - from 154KB to 134KB. I hoped for more, I'm not sure I want to merge this... |
|
These are JS emulators of The rest of the JS support is OK. See https://gist.github.com/pavelsavara/f11dd1078548a16fc923de444f1c9d9b All the symbols in the wasm files are https://gist.github.com/pavelsavara/b7653066a83554168739558630d34847 Among them these caught my eye |
#117813 is the main offender. |
Fixes #122506