Conversation
|
Tagging subscribers to this area: @karelz, @dotnet/ncl |
There was a problem hiding this comment.
Pull request overview
This PR advances the browser-WASM “no sockets” effort by making networking/socket-related components consistently unavailable on the browser target, aligning with the broader PAL cleanup work.
Changes:
- Add browser-specific System.Native stubs for networking, interface addresses, and network statistics.
- Switch browser builds away from syscall-trimming JS hacks and toward native/CMake-based stubbing/linking behavior.
- Add a
-browserTFM forSystem.Net.Socketsthat builds as a PlatformNotSupported assembly.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/libs/System.Native/pal_networkstatistics_wasm.c | Adds WASM/browser stub implementations for network statistics APIs. |
| src/native/libs/System.Native/pal_networking_browser.c | Adds browser stub implementations for System.Native networking/socket entrypoints. |
| src/native/libs/System.Native/pal_interfaceaddresses_browser.c | Adds browser stub implementations for interface/gateway enumeration. |
| src/native/libs/System.Native/CMakeLists.txt | Selects browser-specific PAL sources for System.Native. |
| src/native/eventpipe/ds-ipc-pal-websocket.c | Adds missing runtime helpers for standalone PAL websocket IPC path. |
| src/native/corehost/browserhost/libBrowserHost.footer.js | Removes syscall override hooks previously used to “trim” sockets. |
| src/mono/mono/profiler/log.c | Gates socket header inclusion on configure-time header availability. |
| src/mono/mono/profiler/helper.c | Gates socket-related includes on configure-time header availability. |
| src/mono/mono/profiler/aot.c | Gates socket header inclusion on configure-time header availability. |
| src/mono/mono/mini/cfgdump.c | Gates socket header inclusion on configure-time header availability. |
| src/libraries/System.Net.Sockets/src/System.Net.Sockets.csproj | Adds -browser TFM and marks it as PlatformNotSupported. |
| eng/native/tryrun.browser.cmake | Disables HAVE_SYS_SOCKET_H for browser to avoid emscripten socket emulation. |
| eng/native/configureplatform.cmake | Adds browser CoreCLR link options (-nostdlib + explicit libs). |
b31af63 to
eb1d810
Compare
jkotas
left a comment
There was a problem hiding this comment.
LGTM
It would be nice if folks with more detailed understanding of networking and/or wasm signoff as well.
radekdoulik
left a comment
There was a problem hiding this comment.
Thank you, LGTM
I wonder if we should document the unsupported parts somewhere. @lewing what did we do on mono in such cases?
|
nm, we have https://github.com/dotnet/runtime/blob/main/src/mono/wasm/features.md for mono. I have asked copilot to open draft with similar document for coreclr here #124455 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/mono/mono/mini/cfgdump.c:26
- The include of sys/socket.h is now guarded by HAVE_SYS_SOCKET_H, but netinet/in.h (line 20) and arpa/inet.h (line 26) are still included unconditionally. These headers also won't be available on browser target where HAVE_SYS_SOCKET_H is unset. These includes and the socket-related code that uses them should also be guarded with HAVE_SYS_SOCKET_H or a similar check.
#include <netinet/in.h>
#include <netdb.h>
#include <string.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <arpa/inet.h>
|
/ba-g unrelated CI failures |
Continuation from #123962 (comment)
Contributes to #122506
Browser variant of
System.Net.Socketsnow throw PNSE for all members.SocketAddressPal.Browser.csdoesn't talk to posix and it's shallow similar toSocketAddressPal.Windows.cs