Skip to content

Commit 9c6caef

Browse files
committed
LlamaLoader: skip the nested initialize() that JNI_OnLoad triggers
CI (Smoke test all-backends fat jar, Linux): RpcServer never reported listening within 60 s. RpcServer is the first entry point that loads the library before LlamaModel. Loading runs JNI_OnLoad, whose GetFieldID on LlamaModel initializes that class; its static block called LlamaLoader.initialize() again on the loading thread, and the reentrant lock let a second complete load run: it cleared the temp files and, with a multi-backend jar, extracted and probed every GPU backend again (CUDA, ROCm, SYCL are hundreds of MB) over the library being loaded. - LlamaLoader.runOnceOnThisThread: the nested call returns at once; later calls and other threads still run the load (BackendManifestLoadTest relies on that). Three unit tests pin nesting, re-running and a failed load staying retryable. - smoke-rpc-fatjar.sh waits up to 300 s like the NativeServer smoke and fails when the backend is selected more than once. Reproduced locally with a hand-built multi-backend jar: two selections before, one after; the smoke passes on it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QSXAwbXqx9u5xsMArMvgT8
1 parent dccec67 commit 9c6caef

5 files changed

Lines changed: 104 additions & 15 deletions

File tree

‎.github/smoke-rpc-fatjar.sh‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,22 @@ trap cleanup EXIT
5252
java -cp "$JAR" net.ladenthin.llama.RpcServer --port "$RPC_PORT" --threads 2 --device CPU > rpc-server.log 2>&1 &
5353
PIDS+=($!)
5454
SERVER_PID=$!
55-
for _ in $(seq 1 60); do
55+
# Up to 300 s, like the NativeServer smoke: an all-backends jar extracts every GPU backend's
56+
# library (CUDA, ROCm and SYCL are hundreds of MB) and fails to load each before it reaches one
57+
# this GPU-less runner can load. 60 s was too short for that on the first CI run.
58+
for _ in $(seq 1 100); do
5659
kill -0 "$SERVER_PID" 2> /dev/null || fail "RpcServer exited before listening"
5760
grep -q "RpcServer listening on 127.0.0.1:$RPC_PORT" rpc-server.log && break
58-
sleep 1
61+
sleep 3
5962
done
6063
grep -q "RpcServer listening on 127.0.0.1:$RPC_PORT" rpc-server.log || fail "RpcServer never reported listening"
6164
grep -q "serving \[CPU\]" rpc-server.log || fail "RpcServer --device CPU did not serve exactly the CPU"
65+
# The backend is chosen once. RpcServer is the one entry point that reaches the loader before
66+
# LlamaModel, and JNI_OnLoad initializes LlamaModel, whose static block re-entered the loader and
67+
# ran a second complete load over the library being loaded (LlamaLoader.runOnceOnThisThread).
68+
# A manifest-less jar prints the line zero times.
69+
[ "$(grep -c '\[jllama\] using native backend' rpc-server.log)" -le 1 ] \
70+
|| fail "the native library was loaded more than once: $(grep -c '\[jllama\] using native backend' rpc-server.log) backend selections"
6271
echo "RPC server up: $(grep 'RpcServer listening' rpc-server.log)"
6372

6473
# --- JVM B: the model, offloaded over RPC --------------------------------------------------------

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ from version 5.0.0 onward. Pre-fork releases (`1.x`–`4.2.0`) were authored by
5454
The served devices can be chosen by name (`RpcServer.startLocal(…, List<String> devices)`,
5555
`--device CPU`), because a served device that cannot run an operation terminates the server
5656
(llama.cpp's RPC client reports every operation as supported).
57+
- **`LlamaLoader` no longer loads the native library twice** when the first class to load it is not
58+
`LlamaModel` (e.g. `TextToSpeech`, `LlamaQuantizer`, `RpcServer`): `JNI_OnLoad` initializes
59+
`LlamaModel`, whose static block re-entered the loader and ran a second full load — with an
60+
all-backends fat jar that meant re-extracting every GPU backend over the library being loaded.
5761
- **`.github/verify-native-deps.py`** holds every shipped native library to its known runtime
5862
dependencies (ELF, PE incl. Windows arm64, Mach-O). It found that the macOS dylib has always
5963
linked Homebrew's `openssl@3` (see `TODO.md`).

‎CLAUDE.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -975,6 +975,15 @@ every `supports_op` with `true` (upstream TODO), so a served device that cannot
975975
**aborts the process** — the macOS CI runners' paravirtual Metal GPU has no `MUL_MAT` and took the
976976
first CI run down that way. **Every test that computes over RPC serves `CPU`** (C++ `loopback_server`,
977977
`RpcIntegrationTest`); do not switch one back to the default choice.
978+
**`RpcServer` is the first entry point that loads the library before `LlamaModel`, which exposed a
979+
loader defect** (fixed in `LlamaLoader.runOnceOnThisThread`): `JNI_OnLoad`'s `GetFieldID` on
980+
`LlamaModel` initializes that class, whose static block re-entered `LlamaLoader.initialize()` on the
981+
loading thread — the lock is reentrant, so a second complete load ran while the first was inside
982+
`System.load`, clearing the temp files and, with an all-backends jar, probing and extracting every
983+
backend again over the library being loaded. The fat-jar RPC smoke timed out on it (the doubled
984+
extraction of the CUDA/ROCm/SYCL libraries); it now waits 300 s like the other smoke and fails if the
985+
backend is selected more than once. Only the *nested* call is skipped — later calls still run the body,
986+
which `BackendManifestLoadTest` relies on.
978987
**Single instance per process** (ggml keeps the server state in globals). `startLocal` binds
979988
loopback only; `startOnNetwork` is the explicit, warned opt-in, and binding needs an IPv4 literal
980989
(the server uses `inet_addr`). Endpoints on the client side are `value.RpcEndpoint`

‎llama/src/main/java/net/ladenthin/llama/loader/LlamaLoader.java‎

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ public class LlamaLoader {
6363
private static final Object INITIALIZE_LOCK = new Object();
6464

6565
private static boolean extracted = false;
66+
67+
/** Whether {@link #initialize()} is running; only read and written under the lock. */
68+
private static boolean initializing = false;
69+
6670
private static final LlamaSystemProperties systemProperties = new LlamaSystemProperties();
6771
private static final NativeLibraryPermissionSetter permissionSetter = new NativeLibraryPermissionSetter(System.err);
6872

@@ -123,23 +127,53 @@ private LlamaLoader() {}
123127
* Loads the llama and jllama shared libraries
124128
*/
125129
public static void initialize() {
130+
runOnceOnThisThread(LlamaLoader::load);
131+
}
132+
133+
/**
134+
* Runs {@code body} under the initialization lock, unless this thread is already inside it.
135+
*
136+
* <p>Loading the library re-enters {@link #initialize()} on the same thread: the library's
137+
* {@code JNI_OnLoad} calls {@code GetFieldID} on {@code LlamaModel}, which initializes that class,
138+
* whose static block calls {@code initialize()}. The lock is reentrant, so without this guard the
139+
* nested call ran a second, complete load while the first was still inside {@code System.load}:
140+
* it deleted the extracted files and, with a multi-backend jar, probed every backend again and
141+
* extracted over the library being loaded. Any class but {@code LlamaModel} as the first entry
142+
* point reached it -- {@code RpcServer} hung its fat-jar start that way. Calls from other threads,
143+
* and later calls from this one, still run the body; only the nested one returns at once.
144+
*
145+
* @param body what to run
146+
*/
147+
static void runOnceOnThisThread(Runnable body) {
126148
synchronized (INITIALIZE_LOCK) {
127-
// only cleanup before the first extract
128-
if (!extracted) {
129-
cleanup();
149+
if (initializing) {
150+
return;
130151
}
131-
if ("Mac".equals(OSInfo.getOSName())) {
132-
String nativeDirName = getNativeResourcePath();
133-
String tempFolder = getTempDir().getAbsolutePath();
134-
System.out.println(nativeDirName);
135-
Path metalFilePath = extractFile(nativeDirName, "ggml-metal.metal", tempFolder);
136-
if (metalFilePath == null) {
137-
System.err.println("'ggml-metal.metal' not found");
138-
}
152+
initializing = true;
153+
try {
154+
body.run();
155+
} finally {
156+
initializing = false;
157+
}
158+
}
159+
}
160+
161+
private static void load() {
162+
// only cleanup before the first extract
163+
if (!extracted) {
164+
cleanup();
165+
}
166+
if ("Mac".equals(OSInfo.getOSName())) {
167+
String nativeDirName = getNativeResourcePath();
168+
String tempFolder = getTempDir().getAbsolutePath();
169+
System.out.println(nativeDirName);
170+
Path metalFilePath = extractFile(nativeDirName, "ggml-metal.metal", tempFolder);
171+
if (metalFilePath == null) {
172+
System.err.println("'ggml-metal.metal' not found");
139173
}
140-
loadNativeLibrary("jllama");
141-
extracted = true;
142174
}
175+
loadNativeLibrary("jllama");
176+
extracted = true;
143177
}
144178

145179
/**

‎llama/src/test/java/net/ladenthin/llama/loader/LlamaLoaderTest.java‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,4 +476,37 @@ public void extractFileReplacesACopyWhoseContentDiffers() throws IOException {
476476
assertEquals(target, extracted);
477477
assertArrayEquals(testResourceBytes(), Files.readAllBytes(target));
478478
}
479+
480+
// The nested call is what JNI_OnLoad makes: GetFieldID on LlamaModel initializes it, and its
481+
// static block calls initialize() again on the loading thread. It must not run a second load
482+
// (with a multi-backend jar that deleted and re-extracted the library still being loaded).
483+
@Test
484+
public void aNestedInitializeOnTheLoadingThreadDoesNotRunTheBodyAgain() {
485+
java.util.concurrent.atomic.AtomicInteger runs = new java.util.concurrent.atomic.AtomicInteger();
486+
LlamaLoader.runOnceOnThisThread(() -> {
487+
runs.incrementAndGet();
488+
LlamaLoader.runOnceOnThisThread(runs::incrementAndGet);
489+
});
490+
assertEquals(1, runs.get());
491+
}
492+
493+
@Test
494+
public void aLaterInitializeStillRunsTheBody() {
495+
java.util.concurrent.atomic.AtomicInteger runs = new java.util.concurrent.atomic.AtomicInteger();
496+
LlamaLoader.runOnceOnThisThread(runs::incrementAndGet);
497+
LlamaLoader.runOnceOnThisThread(runs::incrementAndGet);
498+
assertEquals(2, runs.get(), "only a nested call is skipped; tests and later classes re-run it");
499+
}
500+
501+
@Test
502+
public void aFailingBodyDoesNotLeaveTheGuardSet() {
503+
assertThrows(
504+
IllegalStateException.class,
505+
() -> LlamaLoader.runOnceOnThisThread(() -> {
506+
throw new IllegalStateException("load failed");
507+
}));
508+
java.util.concurrent.atomic.AtomicInteger runs = new java.util.concurrent.atomic.AtomicInteger();
509+
LlamaLoader.runOnceOnThisThread(runs::incrementAndGet);
510+
assertEquals(1, runs.get(), "a failed load must be retryable");
511+
}
479512
}

0 commit comments

Comments
 (0)