Repository navigation
Follow-ups to #8: backend fallback, container start, model file cache, float range, chunk depth, poll - #9
Merged
Merged
Conversation
A model whose own backend is gpu, sent without a backend, gets an engine on cpu when the GPU does not start. That engine was kept under the model's choice, "gpu", so a request that then set gpu itself got it: on the stand-in library's gpu-broken.litertlm (header gpu,cpu; no GPU engine starts), "<path>" loaded on gpu, fell back to cpu, and "<path>,gpu" was answered 200 by that cpu engine, nothing created and nothing said. For an LFM2 header, which gets a new engine for each conversation, "<path>,gpu" silently created a new engine on cpu. A backend the user sets must get that backend or an error. Introduced by fix(litert-lm-server): with no backend set, the model's own is used (93b3f70, in #8); main before #8 had no fallback. - engines/litert-lm-server/src/main.cc: an engine's key holds the backend it runs on, and the engine keeps the model's own backend it was made for when no backend was set. A request that sets a backend matches only an engine on that backend, so gpu after a fallback creates an engine on gpu, failing with the GPU's error as a set backend always has, and cpu takes the engine that runs on cpu. A request that sets none still takes the engine made for the same choice of the model's, on whichever backend it fell back to, without trying the GPU again until another engine is loaded; a model that gets a new engine for each conversation gets it on that backend, as before. An engine on a backend a request set, or one made for another choice of the model's (its file replaced), does not serve a request without a backend, which gets the model's own, as before. - BUILDING-ENGINES.md: says a set backend is honoured also then. Behaviour change: after the model's own backend fell back to CPU, a request that sets gpu gets an engine on gpu or its 500, instead of the CPU engine. Tests: in engines/litert-lm-server/model-settings.test.mjs, against the stand-in library: "a backend the request sets after the model's own fell back gets an engine on it or its error, never the other" (gpu set after the fallback, for gpu-broken and for an LFM2 header; cpu set after it; the model's own and a set backend on gpu-first; a file replaced by one whose own backend is gpu). With main.cc as it was it fails (200 and no engine for gpu set after the fallback; a scratch run showed the LFM2 header creating a cpu engine for it). It fails too with only the key changed (requests without a backend then try the GPU again), and with the reuse keyed by whether the model chose (the replaced file's gpu is not used). No existing test changed.
The server refuses to start when its parent is PID 1, which is how it tells that the process that started it died before its PR_SET_PDEATHSIG was armed. But PID 1 is also a container's entrypoint (tini under `docker run --init`, or a shell), which starts the server as its child on purpose, and such a server was refused: under `unshare --pid --fork --mount-proc`, both `sh -c 'litertlm_server serve ... & wait'` and `tini -- litertlm_server serve ...` printed "the process that started turbollm-litertlm-server has already exited; not starting" and exited 1. (A server that is itself PID 1 has parent 0 and was never refused.) This binary ships only in the APK, so this hits host builds run in a container, not phones. Introduced by fix(litert-lm-server): the server ends when the app does, on its own (db0375c, in #8). - engines/litert-lm-server/src/main.cc: a parent of PID 1 means the starter is gone only when PID 1 is the machine's own init, that of the root PID namespace (/proc/self/ns/pid, inode 4026531836, the kernel's PROC_PID_INIT_INO; as before when /proc cannot be read). In a container's namespace the kernel keeps nothing that tells an orphan its PID 1 adopted from that PID 1's child, so both start there. On Android nothing changes: only the app starts this binary, so a parent of PID 1 is always the app gone, also where Android runs in a container of its own (ChromeOS's ARC++). The engine tether, which knows the daemon's pid, and the daemon's sh lifeline are unchanged. - BUILDING-ENGINES.md: says so. Behaviour change: off Android, a server whose parent is PID 1 of a container's PID namespace starts, including one orphaned there before its prctl. Tests: in engines/litert-lm-server/parent-death.test.mjs, where unshare can make a PID namespace (as root, or with -r): "a server whose parent is PID 1 of its own PID namespace, as a container's entrypoint is, starts" (under sh and, where installed, tini; it also ends with that PID 1). With main.cc as it was it fails ("not starting"). "a server whose parent exited before it started does not start", the orphan adopted by this machine's init, passes before and after.
What the server reads from a model's header (which inputs it takes, on which backends, its sampler, context and thinking) was kept for the file's size and modification time in whole seconds. A file rewritten in place within the same second at the same size kept the old file's answers: a stand-in rewritten.litertlm with a text decoder and an embedder, rewritten with a vision encoder instead (same inode, same size, mtime moved from .25 to .75 of the same second), still got "Invalid messages: rewritten.litertlm takes no image input" for an image. And every request opened the file and read its first 32 bytes, even with nothing changed: strace counted 4 opens of the model for 4 requests. Introduced in #8: the size-and-second cache by fix(litert-lm-server): models that take images or audio get encoders (243572a), the read on every request by fix(litert-lm-server): reject out-of-range .litertlm header offsets (c3f51e5). - engines/litert-lm-server/src/main.cc: FileInfo stats the file first and keeps what it read for the file's device, inode, size, and modification and change times to the nanosecond; a request whose file is unchanged costs that stat alone, and one whose file changed reads the header again (the stat comes before the read, so a change in between is read next time). The change time moves with every write and cannot be set back, so a rewrite that puts the old modification time back (cp -p, touch -r, tar) is read again too. A header end the runtime cannot load is still a 500 for every request, now from that one read, and still leaves the loaded engine in place. With --verbose the server logs each read ("Reading the header of <path>"). Behaviour change: a model file changed in any way, also within the same second at the same size or with its modification time put back, is read again; an unchanged one is not. Tests: in engines/litert-lm-server/model-settings.test.mjs, against the stand-in library: "a model file is read once while it is unchanged, and again once it changes, in the same second at the same size or with its time put back" (an image refused, then taken after the rewrite, then refused after a rewrite that restores the modification time to the nanosecond; one read for four requests, two after the rewrite, three after the restore; a header end past 16 KiB refused twice from one read, no engine created after). With main.cc as it was it fails; with only the log line added to it, 4 reads for 4 requests, and the strace run above shows the image still refused after the rewrite. Without the change time compared, the image is taken after the restore. No existing test changed.
LiteRT-LM takes the sampler's temperature and the repetition penalties as floats. The server checked them as doubles (a number, finite, at least 0 for temperature and 1 for repetition_penalty) and converted them, so a value beyond a float's range passed and reached the runtime as an infinity, with a 200: on the stand-in library, temperature 1e39 reached the sampler as inf, and repetition_penalty 1e39, repeat_penalty 1e39, presence_penalty -1e39 and frequency_penalty 1e39 reached the penalty config as infinities. Older bug for temperature: main before #8 has it (temperature passed as a float, checked only to be a number). The penalties came with fix(litert-lm-server): repetition penalties reach the model (b9be217, in #8). - engines/litert-lm-server/src/main.cc: temperature, top_p, top_k, seed and each penalty that does not convert to a finite float is a 400 that names the field, as the other range errors are: "Invalid sampler parameters: ValueError: temperature must be within a float's range (-3.4028235e+38 to 3.4028235e+38), got 1e+39". The test is the conversion itself, so a value a hair past FLT_MAX that still rounds to it (3.4028235e38) is taken, and one that rounds to infinity (3.4028236e38) is not. top_p, top_k and seed were already bounded by their own checks; min_p is never passed to the runtime. - BUILDING-ENGINES.md: says so. Behaviour change: a temperature or penalty beyond a float's range is a 400 instead of an infinity passed to the runtime. Tests: in engines/litert-lm-server/model-settings.test.mjs, against the stand-in library: "a sampler or penalty value a float cannot hold is a 400 that names it, and one it can is passed as set" (1e39 for each field and -1e39 for the two that may be negative, streamed or not, and 3.4028236e38; 3e38, -3e38 and 3.4028235e38 reach the sampler and the penalties as set). With main.cc as it was it fails (200 for temperature 1e39). No existing test changed.
Each chunk the runtime streams is JSON that carries the tool calls it parsed from the model's output. The server parsed it with json::parse, which keeps its own stack, but then copied the tool calls and later wrote them out, which recurse once per level on the connection thread's stack. A stand-in library's chunk holding a tool call 200,000 levels deep killed the server with SIGSEGV (the request got a reset connection, and the loaded model went with the process); 10,000 levels still passed on this machine's 8 MB thread stacks, Android's are about 1 MB. LiteRT-LM 0.18.0 itself stops a JSON tool call at 128 levels (serde_json, rust/json_parser.rs:55) and parses Python and FC syntax recursively on its own thread (parser_common.cc ConvertJsonValue, the copy in data_utils.cc, the dump in c/conversation.cc) before the server sees it, so the crash was shown only with the stand-in's chunk: what this guards is the server's side. Older bug: main before #8 has it (ParseChunk's json::parse). - engines/litert-lm-server/src/main.cc: ParseChunk parses with ParseBounded, the parser a request's body already goes through, now given a depth: the chunk's envelope (the chunk, its tool_calls, the call, its function) is 4 levels, so it takes kMaxJsonDepth + 4 and a call's arguments may nest as deep as a request's, 128 levels. A JSON chunk nested deeper fails its request as on the runtime's own error: the generation is cancelled, and the client gets a 500, or in a stream the response.error event and [DONE], "RuntimeError: the model's output, as the runtime parsed it, is nested deeper than 132 levels (128 within a tool call's arguments)". A chunk that is not JSON is still plain text, as before, however many brackets it opens (json::accept tells, building nothing). - engines/litert-lm-server/testdata/fake_litert_lm.cc: the stand-in answers a message "nested:<n>" with a tool call whose arguments nest n arrays, and "nested-text:<n>" with a chunk of n brackets and text. Behaviour change: a runtime chunk nested deeper than 132 levels fails its request instead of crashing the server (or, below the stack's limit, being passed on). Tool calls the runtime takes are passed on as before: with LiteRT-LM 0.18.0, Qwen3-0.6B and a regex response_format forcing arguments 125 and 126 levels deep, 200s before and after (127 and more the runtime refuses itself). Tests: in engines/litert-lm-server/model-settings.test.mjs, against the stand-in library: "model output nested deeper than the server reads fails its request, streamed or not, and the server stays up" (100, 125 and 127 arrays still a tool call; 128 and 200,000 a 500 and a stream's error event; a non-JSON chunk of 200 brackets still text; a chat answered after). With main.cc as it was it fails (fetch failed: the server died with SIGSEGV); it fails too with the bound counting the envelope (125 a 500), and without the accept() check (the non-JSON chunk a 500). No existing test changed.
The engine service polls the daemon every 3 s: GET /api/v1/downloads and GET /api/v1/activity, and GET /api/v1/status while nothing runs, each request on a connection of its own after a proof of the daemon on another (DaemonAuth): 4 loopback connections a tick, 6 while idle, for as long as the app runs. The proof has to stay per request, so only fewer requests save connections, and the activity route carried nothing of the other two. TurboLLM's fix(api): one activity request carries what the Android poll reads adds them to it: `downloads`, the running and queued rows of GET /api/v1/downloads, and `status`, the fields of GET /api/v1/status the app reads, in the same names. Introduced in #8: main before it polled the activity alone, on a pooled connection with no proof; the downloads by fix(android): the download notification shows its progress and speed, the status by fix(android): the notification says what is running, and the proof per request by fix(android): app proof and secret each go on a new socket, never pooled. - composeApp/src/androidMain/kotlin/com/turbollm/app/EngineService.kt: each poll reads the activity first, and takes the downloads and the status from that reply when it carries them as those routes answer (ActivityReply), asking a route only for what the reply lacks: one request, two connections a tick, with a daemon that has them; with an older one, the same requests as before. A close's check (anythingRuns) reads them from its one activity reply the same way. The notification, the wake lock and what a close decides are as before. - composeApp/src/androidMain/kotlin/com/turbollm/app/DownloadNotice.kt: the running and queued rows are read from a reply's JSON object (liveDownloads), so the activity reply's list goes through the same filter as GET /api/v1/downloads's. - PLAY-FGS-DECLARATION.md, play-store-assets/privacy-notes.md: the `serving` field, and the requests, can come from the activity reply. Behaviour change: with a daemon that has TurboLLM's fix(api): one activity request carries what the Android poll reads, the poll makes one request a tick instead of two or three. Tests: there is no Kotlin test harness in this repo, so none is added. Checked on the JVM with the real EngineService.kt and DownloadNotice.kt against the scripted daemon of fix(android): leaving the app no longer unloads the model, as later commits extended it, its activity answer built as that TurboLLM commit builds it when asked (its downloads and status from the scripted routes, each left out when its route answers no JSON). With it, every poll is one request, two connections: idle with a model loaded or with none, a reply, an approval wait, a download running, queued or paused; before the change, two requests (four connections) and three while idle. The status unreadable, only it is asked of its route (four connections), both unreadable, both (six); an activity reply whose fields are not those routes' answers asks the routes. The shade and the wake lock read the same in each case. All 150 scenarios of the harness, notifications off and on (300 runs), give the same verdicts and shade with an older daemon as before; with the new one, too, but for three whose events were timed to land inside a close's three-request second reading: with one request a reading, the close is decided 4.5 s sooner, before they come, and timed inside its one-request second reading they keep the app in the foreground as before (a reopen, a swiped card then a reopen, a load starting). compileDebugKotlinAndroid and mergeDebugJniLibFolders pass. Not run on a device.
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
ciroprogamer
had a problem deploying
to
play-alpha
October 26, 2026 18:00 — with
GitHub Actions
Error
ciroprogamer
force-pushed
the
feat/modest-fermi-kat44x
branch
from
October 26, 2026 18:22
735d0d1 to
1e7ccdf
Compare
ciroprogamer
had a problem deploying
to
play-alpha
October 26, 2026 18:22 — with
GitHub Actions
Error
…a new engine "fix(litert-lm-server): a set backend never gets the fallen-back engine" keys an engine by the backend it runs on, so after a model's own gpu fell back, its key says cpu and its backend_chosen gpu. A request without a backend reuses an engine made for the same choice of the model's (backend_chosen), and that commit says one made for another choice, which only a replaced file brings (the choice is its header's), does not serve it. But the plain key comparison never looked at the choice: replace a file whose own gpu fell back to cpu with one whose own backend is cpu, and a request without a backend matched the old engine's key (cpu) and was served by the engine loaded from the old file, with the old file's ModelFileInfo (sampler, context window, recurrent state). For an LFM2 file replacing a generic one, no request got the new engine per conversation that LFM2 needs. Before that commit the key said gpu and the request loaded the new file. The commit's own test replaced a file in the other direction only. - litert-lm-server/src/main.cc: GetOrCreateEngine reuses no engine, and makes no fresh-state copy of one, whose backend_chosen is another than a backend-less request's choice; it is released and the new file loaded. Not changed: a file replaced by one of the same choice, or used with a backend set, still keeps the old file's engine and its ModelFileInfo, as before that commit: the engine's key holds no version of the file. The comment says so. Tests: model-settings.test.mjs "a backend the request sets after the model's own fell back gets an engine on it or its error, never the other" now also replaces a gpu,cpu file whose gpu does not start with an LFM2 one listing cpu, and expects a cpu engine for it and another for its next conversation: it fails on the previous main.cc (no engine created). Host build with -Wall -Wextra: 0 warnings.
…by design
"fix(litert-lm-server): a container's init may start the server" lets
a server whose parent is PID 1 of a PID namespace other than the root
one start, as a container's entrypoint starts it. It left "a server
whose parent exited before it started does not start" deciding only
on ppid 1, so run inside any container (Docker, a devcontainer,
`unshare --pid`), where the orphaned sh is adopted by the namespace's
PID 1, the test went on, the server started as designed, and the test
failed ("the server started"). The file passed there before.
- parent-death.test.mjs: that test is skipped, saying why, when this
process is in a PID namespace other than the root one (the server is
in the same one); an unreadable /proc counts as the root one, as the
server takes it.
- BUILDING-ENGINES.md: the orphan check is said to run outside a
container's PID namespace.
Tests: parent-death.test.mjs, 4 pass in the root namespace; under
`unshare --pid --fork --mount-proc` 3 pass and the orphan test is
skipped, where it failed before.
ciroprogamer
had a problem deploying
to
play-alpha
October 26, 2026 22:30 — with
GitHub Actions
Error
ciroprogamer
force-pushed
the
feat/modest-fermi-kat44x
branch
from
October 26, 2026 22:38
5ae78cd to
c8288ee
Compare
ciroprogamer
requested a deployment
to
play-alpha
October 26, 2026 22:39 — with
GitHub Actions
Waiting
mohitsoni48
approved these changes
Oct 31, 2026
mohitsoni48
left a comment
Owner
There was a problem hiding this comment.
Reviewed (independent Opus review, no merge-blocking findings). Notes for follow-up, not blocking: an explicit 'gpu' request after a GPU->CPU fallback drops the working CPU engine before retrying the GPU (main.cc:917-920), and the Kotlin poll change has no test in the repo (the PR's JVM harness is not part of the diff).
This branch is waiting to be deployed
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
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.
Summary
Follow-ups to #8: the nits listed when it was approved. One commit per item, each with a regression test that fails without it (the poll change is checked in the JVM harness earlier commits used; this repo has no Kotlin tests). Rows 7 and 8 came from this PR's own review: a fix to row 1, and a test row 2 left failing inside containers.
fix(litert-lm-server): a set backend never gets the fallen-back engine,gpusilently got the CPU engine (or, for LFM2, a new CPU one). The key now holds the backend the engine runs on: a set backend gets an engine on it or that backend's error, and a request without one keeps the fallen-back engine.fix(litert-lm-server): a container's init may start the servergetppid()==1orphan check refused a server whose legitimate parent is a container's PID 1 (sh -c 'srv & wait',tini -- srv). Off Android, a parent of PID 1 counts as orphaning only in the root PID namespace; Android keeps the strict check.fix(litert-lm-server): a rewritten model file is read againFileInfowas keyed on size and mtime in seconds and every request opened and read the header. It is now keyed on device, inode, size and the nanosecond modification and change times from one stat, so a same-second rewrite (or one that puts the mtime back:cp -p,touch -r) is read again, and an unchanged file is read once.fix(litert-lm-server): a value no float can hold is a 400float, so 1e39 reached the runtime as infinity. Temperature and the four penalties beyond a float's range are now a 400 naming the field.fix(litert-lm-server): runtime chunks are parsed with a depth boundParseChunkparsed the runtime's chunks unbounded, and copying and writing out deeply nested tool calls recursed per level (a stand-in chunk of 200,000 levels killed the server). Chunks now go throughParseBoundedwith the request bound plus the chunk's 4-level envelope, so tool calls the runtime accepts pass as before (checked on LiteRT-LM 0.18.0 with Qwen3-0.6B at 125 and 126 levels) and a deeper chunk fails its request cleanly; a non-JSON chunk is still text.fix(android): the engine poll asks the daemon once a tickdownloadsandstatusfields TurboLLM'sGET /api/v1/activitycarries (mohitsoni48/TurboLLM#294), one request and two connections a tick, and falls back to/api/v1/downloadsand/api/v1/statuswhen a field is missing (an older daemon).fix(litert-lm-server): a file replaced by one of another choice gets a new enginetest(litert-lm-server): the orphan test skips where an orphan starts by designunshare --pid) it failed. It is now skipped there, saying why, and BUILDING-ENGINES.md says the orphan check runs outside a container's PID namespace.Also from that list, handled elsewhere or already done:
/v1/messagesmapping could drop a user turn holding only a document and end on the assistant's turn; Follow-ups to #287: model scan, fetch_url, bench VRAM, desktop, gateway, Android poll TurboLLM#294 fixes that.shouldInterceptRequest, so a page could reach another loopback port with the app cookie. Closed by aconnect-srcin the daemon'sAPP_SECRET_CSP(Follow-ups to #287: model scan, fetch_url, bench VRAM, desktop, gateway, Android poll TurboLLM#294).DaemonAuth.kt:88reads the proof until EOF: already bounded: the proof is read withreadAtMost(..., PROOF_MAX_BYTES)underPROOF_DEADLINE_MS. The answers to the secret-carrying requests after a proof are read withreadText()uncapped; whoever answers them already holds the secret, and capping them would mean replacing HttpURLConnection for all ofopen()'s callers, so it is left as is.Behaviour changes worth knowing
gpugets a GPU engine or the GPU's error, not the CPU engine (1); a request without a backend gets a new engine for a file replaced by one whose own backend is another, even the one the old engine fell back to (7).Needs a decision (not changed here)
FileInfoalready reads would reload it on any change to the file, atouchincluded.Merge order and Play Console
Device checks
connect-src 'self'as coveringws://127.0.0.1:<own port>).Testing
At row 6's commit:
-Wall -Wextranode --test --test-concurrency=1 scripts/*.test.mjs engines/litert-lm-server/*.test.mjswith LiteRT-LM 0.18.0, LFM2.5-230M, gemma-3-270m-it, Qwen3-0.6B (thinking) and LFM2.5-VL-450M (vision), and the NDK:composeApp:compileDebugKotlinAndroid :composeApp:mergeDebugJniLibFoldersEach commit's stand-in tests pass at that commit and fail without its fix. Also run: the backend fallback with 12 concurrent mixed requests; the PID-namespace cases under
unshare --pid --forkwith sh and tini; strace counts of header reads and a rewrite stress run; the float range edges around FLT_MAX; deep tool calls through the real runtime; the poll in the JVM harness (150 scenarios, notifications off and on, with an older and a new daemon, plus missing and malformed fields).Rows 7 and 8, at the last commit, without the real runtime or the NDK (neither was at hand); neither row touches Kotlin or the NDK build:
node --test --test-concurrency=1 scripts/*.test.mjs engines/litert-lm-server/*.test.mjs: 92 tests, 77 pass, 15 skipped (the real-model ones), 0 fail.-Wall -Wextra: 0 warnings.parent-death.test.mjsunderunshare --pid --fork --mount-proc: 3 pass, the orphan test skipped (it failed there before 8).main.cc(no engine created for the replaced file).