fix: Keep uni_curl_shim.c buildable on Windows so downstream Native builds link#640
Merged
Conversation
…uilds link The dlsym-based rewrite that fixed #622 reached for <dlfcn.h> and <pthread.h> unconditionally. MSVC ships neither, and Scala Native compiles this file into every downstream binary, so every Windows Scala Native consumer of uni 2026.1.17 now dies with "fatal error: 'dlfcn.h' file not found" — wvlet's Windows native build among them, even though it never calls curl. Split the lookup and the run-once init per platform, keeping the property #622 turned on: the object file must reference no libcurl symbol. Windows has no RTLD_DEFAULT, so walk the loaded modules with EnumProcessModules and try GetProcAddress on each; pair it with InitOnceExecuteOnce in place of pthread_once. uni has no Scala Native build on Windows, which is why nothing here caught this. Rather than stand up a full Windows Native toolchain, compile the shim standalone with clang on all three platforms and assert via nm that it names no curl_easy_* symbol. That guards both regressions this file has now had, and needs no Scala Native. The paths-filter never watched **.c either, so a shim-only edit skipped CI entirely; it does now.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces Windows support to the uni_curl_shim.c file, which dynamically resolves libcurl symbols at runtime to prevent link-time errors in downstream Scala Native projects that do not use CurlBindings. On Windows, it utilizes EnumProcessModules and GetProcAddress for symbol lookup, and InitOnceExecuteOnce for thread-safe initialization, while POSIX platforms continue to use dlsym and pthread_once. Additionally, a new CI script check-curl-shim.sh is added to verify that the shim compiles cleanly and does not reference any libcurl symbols directly. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…nly one A standalone clang compile of uni_curl_shim.c on three OSes only ever tests one file. Building Scala Native on Windows for real covers the same break and every future one, and running the native suite there exercises the shim's Windows symbol lookup at runtime rather than merely compiling it. The nm check survives as one step in the Linux Native job, because no Scala Native job can replace it: build.sbt passes -lcurl unconditionally, so uni's own binaries always resolve curl_easy_setopt no matter what the shim declares. Only a consumer that never links libcurl breaks — which is exactly how #622 escaped this repo's CI and landed in wvlet.
… all The docs gave this one line: "Scala Native requires libcurl to be available at runtime". That leaves out the two things a Native user actually trips over. First, curl-free binaries need no libcurl — that is what #622 bought, and nobody would guess it from the old wording. Second, libcurl must now be a shared library: runtime symbol lookup finds nothing in a static archive, so a statically linked libcurl builds fine and aborts on the first request. Windows needs a step beyond installing it, since Scala Native asks the linker for curl.lib while vcpkg writes libcurl.lib.
…mport libs Scala Native's javalib ships its own z.c that includes <zlib.h>, so the build needs the vcpkg include directory on the header search path as well. Without it the job dies compiling z.c, well before it reaches anything of uni's. Worth noting what that run did prove: it compiled uni_curl_shim.c on MSVC without complaint, which is the regression this job exists to catch.
Member
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces Windows compatibility to the uni_curl_shim.c file, which lazily resolves libcurl symbols at runtime to prevent link errors in downstream Scala Native projects that do not use CurlBindings. It implements a platform-specific split using POSIX APIs (dlsym and pthread_once) on Unix-like systems and Windows APIs (EnumProcessModules, GetProcAddress, and InitOnceExecuteOnce) on Windows. Additionally, a new CI check script check-curl-shim.sh is added to verify that the compiled shim does not reference libcurl symbols, and the documentation and ADRs are updated to reflect these changes and the requirement for dynamic linking. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Scala Native's javalib links -lcrypto and -lssl, which clang in MSVC mode asks for as crypto.lib / ssl.lib while vcpkg writes libcrypto.lib / libssl.lib. The link died on LNK1104: cannot open file 'crypto.lib'. The compile stage is now fully clean, uni_curl_shim.c included, which is the regression this job is here to catch.
…y off Nearly all of that job's wall clock is toolchain setup, and almost no pull request can break what it checks. Run it on every push to main, and on pull requests only when they touch native code: a .c file Scala Native compiles into every downstream binary, anything under a .native/ tree, or the Scala Native version in project/. Its own workflow file counts as native too, so changes to the job still test the job. dorny/paths-filter matches with dot:true, so **/.native/** does reach the dot-prefixed source trees.
The link wanted zlib.lib; vcpkg's zlib port installs z.lib. This build asks for -lz while Scala Native's javalib asks for -lzlib, so both names have to resolve. Take the first candidate that exists for each name rather than hardcoding one mapping, since the vcpkg ports rename these across revisions, and say so out loud when nothing matches instead of failing 200 seconds later at the link step.
…ative there
A full Windows Native job is the guard I wanted, and it does not work. It brings up
LLVM, vcpkg's libcurl/zlib/OpenSSL and every foo.lib alias that -lfoo asks for,
compiles all sources, then fails to link:
error LNK2019: unresolved external symbol scalanative_pollin
referenced in function ...wvlet.uni.http.NativeServerTest...
NativeServer uses POSIX poll(), and Scala Native's posixlib guards poll.c behind
__unix__ || __APPLE__. Every native HTTP test starts a server, so uni's native test
binary cannot link on Windows at all. Give NativeServer a WSAPoll path and this
becomes possible; until then, compiling the file with the same clang/MSVC toolchain
is what Windows coverage there is.
That discarded run was not wasted: it linked everything except poll, so none of the
unresolved symbols were curl_easy_* — the shim compiles and links clean on MSVC.
2 tasks
xerial
added a commit
that referenced
this pull request
Jul 8, 2026
…tive CI (#641) **Description** uni's Scala Native test binary cannot link on Windows — at all, today, on `main`: ``` error LNK2019: unresolved external symbol poll error LNK2019: unresolved external symbol scalanative_pollerr error LNK2019: unresolved external symbol scalanative_pollhup error LNK2019: unresolved external symbol scalanative_pollin error LNK2019: unresolved external symbol scalanative_pollnval referenced in function ...wvlet.uni.http.NativeServerTest... fatal error LNK1120: 5 unresolved externals ``` Scala Native's [`posixlib/poll.c`](https://github.com/scala-native/scala-native/blob/v0.5.12/posixlib/src/main/resources/scala-native/poll.c) wraps its whole body in `#if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__))`, so `poll` and the `scalanative_poll*` constant accessors don't exist there. `NativeSocket` referenced `scalanative.posix.poll`, every native HTTP test starts a server, and so the whole binary was unlinkable. That gap is why uni had **no Windows Scala Native CI**, and therefore why #640's root cause — v2026.1.17 shipping a POSIX-only `#include <dlfcn.h>` in `uni_curl_shim.c` — broke every downstream Windows build instead of failing here. **The fix.** Exactly three calls differ on Windows. Everything else uni uses (`socket`, `bind`, `listen`, `accept`, `recv`, `send`, `setsockopt`, `shutdown`) posixlib already maps onto winsock — the failed link proved it, by resolving all of them and only missing `poll`. So `uni_socket_shim.c` takes those three, behind an `@extern object SocketShim`: | | POSIX | Windows | |---|---|---| | `uni_socket_startup()` | nothing | `WSAStartup`, once, via `InitOnceExecuteOnce` | | `uni_socket_wait_readable(fd, ms)` | `poll` | `WSAPoll` | | `uni_socket_close(fd)` | `close` | `closesocket` | Three things here are easy to get wrong, so they're in the [ADR](https://github.com/wvlet/uni/blob/fix/native-server-windows-poll/adr/2026-07-08-native-socket-shim.md): - **The split has to be in C.** Scala Native has no per-OS source directory (`.native` is per-*platform*), and DCE keeps every *reachable* branch — so a runtime `if (isWindows)` in Scala links both sides and still fails on `scalanative_pollin`. Merely *referencing* `posix.poll` is the break. (`scalanative.windows.WinSocketApi.WSAPoll` is the mirror-image problem: referencing it breaks the POSIX link on `ws2_32`.) - **`#pragma comment(lib, "ws2_32.lib")`, not `@link("ws2_32")`.** Scala Native compiles this `.c` into every downstream binary, including ones that never open a socket. A bare `WSAPoll` reference with no guaranteed `-lws2_32` breaks those links — precisely the #622 trap. The pragma embeds the dependency in the object's linker directives, so it travels with the object; a Scala-level `@link` gets dropped by DCE. Scala Native's own `posixlib/sys/socket.c` does exactly this. - **`WSAStartup` had no other caller.** Winsock rejects every `socket()` with `WSANOTINITIALISED` until it runs. Scala Native calls it from `WinSocketApiOps.init()`, reached only by javalib's `java.net` — which uni's posixlib sockets never touch. And `close()` on a socket *links* on Windows (`oldnames.lib` → `_close`) but only knows CRT file descriptors, so it silently leaks the socket; that applies to the error-cleanup paths in `bindAndListen`/`connect` too, not just the public `close`. **Verification.** - The shim's contract is exercised directly by a C harness over a `socketpair`: idle → timeout, data pending → readable, drained → timeout, **data+hangup → readable** (drain before reporting hangup), hangup-only → readable/EOF, closed fd → error. All pass, and `uni_socket_startup()` is idempotent. - `projectNative/test` passes locally on macOS: 65/65, including `NativeWebSocketClientTest`'s heartbeat test, which is the poll-timeout path. - CI now runs a real **`Scala Native (Windows)`** job — restored in place of #640's standalone `clang` compile, which was only ever a stand-in for this. It exercises `uni_socket_shim.c` (`WSAStartup`/`WSAPoll`/`closesocket`) *and* `uni_curl_shim.c` (`GetProcAddress` over `EnumProcessModules`) at runtime, not just at compile time. Gated as before: every push to `main`, plus PRs touching native code. `check-curl-shim.sh` stays as a step in the Linux Native job. It is still irreplaceable: `build.sbt` passes `-lcurl` unconditionally, so no Scala Native job on any OS can notice the curl shim regrowing a libcurl symbol reference — only a consumer without `-lcurl` breaks. Its Windows/`llvm-nm` branch is dropped now that the real job covers Windows compilation. **Related Issue/Task** Follow-up to #640. Removes the "a real Windows Native job is impossible" caveat that PR had to record, and closes uni's Windows Scala Native coverage gap at its source. **Checklist** - [x] This pull request focuses on a single task. - [x] The change does not contain security credentials 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
uni 2026.1.17 broke every downstream Scala Native build on Windows:
The
dlsymrewrite that fixed #622 reached for<dlfcn.h>and<pthread.h>unconditionally, and MSVC ships neither. Because Scala Native compiles every.cunderresources/scala-native/from every jar on the classpath, this hits consumers that never touch curl — wvlet's Windows native build is failing onmainfor exactly this reason, and it only linkswvc-lib. Both Windows jobs there die on this one error and nothing else.The fix. Split the two POSIX primitives per platform, preserving the property #622 turned on — the object file must reference no libcurl symbol, or downstream links without
-lcurlfail:dlsym(RTLD_DEFAULT, name)GetProcAddressoverEnumProcessModulespthread_onceInitOnceExecuteOnceWindows has no all-modules handle, so the shim enumerates the process's loaded modules and tries each — the process image first, matching
dlsym's order.#define PSAPI_VERSION 2keepsEnumProcessModulesinkernel32(asK32EnumProcessModules) rather thanpsapi.lib, since a jar-resource.ccan't make downstream binaries add a linker flag. The POSIX path is unchanged in behaviour.One consequence worth flagging: runtime lookup only finds exported symbols, so libcurl must be linked as a DLL, not a static
.lib. vcpkg's defaultx64-windows/arm64-windowstriplets already do that. This isn't new to Windows — a staticlibcurl.ais equally invisible todlsym(RTLD_DEFAULT, ...). It's the price #622's fix already paid; the ADR and the user docs now say so out loud.Why CI didn't catch it, and what now does. Two distinct failure modes, neither check subsuming the other:
curl shim C (Windows)— compiles the shim standalone with clang onwindows-latest. Nothing else here compiles the Windows half of the#if.check-curl-shim.sh— one step in the Linux Native job, asserting vianm -uthat the object names nocurl_easy_*symbol. No Scala Native job can catch that on any OS:build.sbtpasses-lcurlunconditionally, so uni's own binaries always resolve those symbols and link happily. Only a consumer that doesn't link libcurl breaks — which is precisely how ScalaNative - unable to build when using uni::2026.1.13 (works with uni::2026.1.12 and earlier) #622 escaped this repo's CI. Inspecting the object stands in for that consumer.I checked (2) bites rather than rubber-stamps: against
v2026.1.16'sextern-based shim it reports_curl_easy_setopt/_curl_easy_getinfoand exits 1, reproducing #622.Both are gated on a new narrow
nativepaths-filter (**.c,**/.native/**,project/**, the workflow itself), so they run on every push tomainand only on PRs that can actually break them. (dorny/paths-filtermatches withdot: true, so**/.native/**does reach the dot-prefixed source trees.) Thechangesfilter never watched**.cat all before, so an edit touching only this file skipped CI outright.Important
A real
Scala Native (Windows)job would be the better guard, and uni cannot run one. I built it — LLVM, vcpkg libcurl/zlib/OpenSSL, aliasing everyfoo.libthat-lfooasks for — and it compiles everything, then fails at the final link:NativeServeruses POSIXpoll(), and Scala Native'sposixlib/poll.cis wrapped in#if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__))— those symbols don't exist on Windows. Every native HTTP test starts a server, so uni's native test binary is unlinkable there, full stop. GivingNativeServeraWSAPollpath would unblock it; this PR's history has the working toolchain setup to reuse. Worth filing as a follow-up issue.That discarded run wasn't wasted: it linked everything except
poll, so none of the unresolved symbols werecurl_easy_*— proving the shim compiles and links clean under clang/MSVC, which is the regression this PR is about.Docs. The old text said only "Scala Native requires libcurl to be available at runtime." Added Linking libcurl on Scala Native covering the three things a Native user actually hits: you need libcurl only if you use the HTTP client (DCE drops
@link("curl")otherwise — that's what #622 bought, and nobody would guess it); it must be a shared library; and on Windows you aliaslibcurl.libto thecurl.libScala Native asks for.Related Issue/Task
Follow-up to #622 / the ADR added in #631. Unblocks wvlet's
Nativeworkflow onmain.Checklist
🤖 Generated with Claude Code