Server-side backend: a native, JVM-free runtime for Codename One handlers - #5741
Open
shai-almog wants to merge 283 commits into
Open
Server-side backend: a native, JVM-free runtime for Codename One handlers#5741shai-almog wants to merge 283 commits into
shai-almog wants to merge 283 commits into
Conversation
A virtual thread registered after the collector's once-per-cycle snapshot is invisible to the stack scan until the next cycle. Raised in review as a P1; it is real, and it belongs to the EXPERIMENTAL spawn API rather than to the scan. Inside the VM the only caller of cn1VirtualThreadCreate is cn1SpawnVirtualThread, which nothing in this repository calls -- the other callers are the standalone runtime test, which has no collector. Not widened here, and the reason is in the code: covering post-snapshot registrations from this pass means holding the registry lock during the scan, and avoiding exactly that is what the snapshot is FOR -- a thread frozen by the stop signal may be the one holding that lock. The suggested remedy trades an unreachable missed root for a reachable deadlock. Listed as the fourth known gap above cn1SpawnVirtualThread. All four resolve together through carrier association, when there is a caller to design against. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Same defect as the one already fixed in VirtualThreadRuntimeTest, in the other test this branch adds: output was read inline before waitFor, and that read blocks until the child closes stdout. A program that HANGS -- one of the regressions this test exists to catch -- therefore never reached the timeout, and the job would sit until CI killed it rather than failing here. A timeout that the guarded failure prevents from being evaluated is not a timeout. Swept for it rather than fixing the reported line alone, and the sweep narrowed the scope rather than widening it: 26 places in the suite read process output before waitFor, but 24 of them use the UNTIMED waitFor(), where a blocking read is equivalent and there is no timeout to defeat. Only the two tests added by this branch pass a timeout, and both are now drained on a separate thread with a bounded join. Nothing else needs changing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
stringToUTF8 returns threadStateData->utf8Buffer -- one buffer per thread, reused -- so converting dest overwrote the source and rename(p, d) was rename(d, d). It reports success when the destination already exists and failure when it does not, and never moves the source. Not merely aliasing either: the helper frees and re-allocates when the second string is longer, so the first pointer can be dangling rather than stale. Two corrections to how this was reported. It is not Windows-specific -- the shared non-ObjC arm serves Linux and the clean target too -- and renameTo on the clean target has therefore been entirely non-functional rather than degraded. The source is copied out before the second conversion now. Swept before fixing: this is the ONLY function in java_io_File.m, nativeMethods.m or cn1_globals.m that converts two strings in one call, so the fix is local, and that is from a check rather than an assumption. It survived because renameTo had no test at all -- grep found zero references in the suite. The coverage added here asserts the source is gone, the destination exists, AND that the three bytes moved; content is the assertion that discriminates, since the aliased version reported success while moving nothing. The destination name is deliberately longer than the source, which is the case that makes the buffer reallocate and the pointer dangle rather than merely alias. Verified by reverting: 5/5 fail against the aliased version, 5/5 pass with the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fixing the unmapped REFERENCE case earlier in this branch, I made appendJsonUsing quote instance.toString() when no mapper is found. That is right for a reference field, where emitFieldToMap stores _v.toString(). It is wrong for a list ELEMENT, where emitFieldToMap stores _e unchanged and the writer keeps its JSON type -- so a List<Object> holding 5 serialised as ["5"] instead of [5]. Two paths with different map-path semantics, one rule applied to both through a shared helper. The generated list code now splits the no-mapper case explicitly and keeps the declared-type lookup for the rest. Covered: the parity test carries a List<Object> of a number, a boolean and a string, and pins "mixed":[5,true,"s"]. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both stream classes are new in this branch and neither had a reclamation hook, so a stream that went unreachable unclosed held its FILE* until the process exited. On a desktop app that is untidy; on a long-running clean-target server it ends in EMFILE, and for output it also drops whatever was still buffered. finalize() is the established convention here rather than an invention -- java.lang.Thread already releases its native thread state the same way, and this VM runs finalizers for exactly this purpose. Deliberately silent: a finalizer has nobody to report to, and throwing from one is worse than the leak it is cleaning up. close() remains the way to learn that a close failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t cap Three review findings, and they get three different answers. A SINGLE LEADING SEPARATOR IS NOT ABSOLUTE ON WINDOWS. "\logs\app.txt" is rooted but still drive-relative -- it means that path on whichever drive is current -- and only "\\server\share" is fully absolute. Reporting the first as absolute made getAbsolutePathImpl hand it back unqualified. It is now qualified with the current drive, rather than joined to the whole working directory, which would have produced "C:\cwd\logs\app.txt". CLOSING TWICE IS NO LONGER FATAL. Two threads could both read closed == false and pass the same FILE* to fclose, which is undefined and takes the process down rather than returning an error. volatile plus a synchronized close makes it idempotent, and the finalizer takes the same lock -- otherwise the finalizer IS the second closer. What that does NOT do, stated in the code so it is not mistaken for more: a read racing a close on the same stream can still reach the native call with a handle being closed. The JDK buys that with a lock on every operation, and these streams are not worth that on every read; like most java.io streams they are for one thread at a time. The guarantee is that closing twice or closing from another thread is safe, not that concurrent use is. THE SNAPSHOT CAP IS LISTED, NOT FIXED. Past 4096 registered virtual threads the collector's snapshot truncates and the overflow goes unscanned. Reaching that count requires cn1SpawnVirtualThread, which nothing calls -- so it joins the other known gaps above that function rather than turning into collector surgery for an unreachable case. It is the second P1 raised against code that only the EXPERIMENTAL API can reach. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…write
The report named fflush. Sweeping the file layer found five unparked blocking
calls rather than one, and the two I would not have thought of are the opens.
- fflush pushes the buffer at the peer and blocks exactly where the write does.
- Both fclose calls FLUSH before closing, so they block in the same place.
- Both fopen calls block on a FIFO: opening for read waits until a writer opens
the other end, opening for write waits for a reader, and there may never be
one. Opening reads as cheap, which is precisely why it was missed.
Each left the VM thread active while it blocked, so a collection waited for a
safepoint that could not arrive -- and on Windows, where CN1_GC_CAN_FORCE_STOP is
off, there is no escalation to break that wait.
The opens need no buffer keep-alive, unlike the reads and writes: `path` points
into the thread's utf8Buffer, which is C memory a collection cannot move or
reclaim, whereas those hold an interior pointer into a Java array the collector
could sweep.
Verified by re-running the same sweep afterwards: all eight java_io_* natives that
touch stdio now park.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Half of this report is right and half of it is not.
THE EMBEDDED NUL IS REAL. The native converts to a C string, where a NUL ends it,
so a lookup of "PATH" + NUL + "suffix" found PATH and returned that variable's
value. A silent answer about a DIFFERENT variable is worse than reporting the name
unset, so a name containing a NUL is now answered null.
The check is in Java because that is where the information is: one indexOf against
re-deriving the byte length in C and walking the string's backing representation,
which is the compact byte[] versus char[] distinction consolidated earlier in this
branch. That moved the null check up too, so the native is now the raw lookup.
ILLEGALARGUMENTEXCEPTION IS DECLINED. Neither this VM's contract for getenv ("or
null when it is not set") nor java.lang.System.getenv(String) declares it -- the
documented exceptions are NullPointerException and SecurityException. The
validation that throws IllegalArgumentException belongs to ProcessBuilder's
environment mutation, not to a lookup. An empty name, or one containing '=', names
nothing, and null is exactly what "not set" means. Adding the throw would make this
VM diverge from the platform in the name of matching it. Written at the method so
it is not re-raised.
The rename to getenvImpl is the dangerous part of this edit -- a wrong native name
compiles, links, and silently drops the method, leaving a green build and an inert
feature. check-native-signatures.sh reports 0 fatal with every native resolving on
both ports, and the UTF-8 environment test passes end to end, which it could not if
the symbol had been dropped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TWO THINGS, and the first is a correction to my own sweep. Last round I said every java_io_* native touching stdio now parks, and verified it mechanically -- against a pattern list I had built from the calls I had already fixed. It did not include ftell or fseek, so skipImpl and availableImpl were still unparked. A mechanical check is only as good as the pattern given to it. Seeking is not free everywhere: a remote mount or a FUSE filesystem services ftell/fseek over the wire, and the thread sits inside the CRT for the duration -- where a collection waits for a safepoint that cannot arrive, with no forced-stop escalation on Windows to break it. Both functions take ONE yield spanning their whole seek sequence rather than bracketing each call: the collector only needs the thread parked, and three yield/resume pairs would cost more than the seeks they guard. Re-swept with ftell/fseek included: zero unparked. THE THREAD BENCHMARK NEVER TERMINATED. ThreadCost spawns non-daemon threads parked on LOCK.wait() and nothing ever notified them, so returning from main ended only the main thread. The documented "/usr/bin/time -l /tmp/threadcost" invocation could not print its result without an external kill -- the measurement was taken and then discarded. It notifies after measuring; waking the workers cannot affect a number already recorded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Secures work that only exists in this checkout: BackendTestSupport, whose absence silently made 21 backend tests uncompilable rather than failing them, the fasthttp comparison arm, and the GC pause/stress demos. It was lost once already and re-recovered from a dangling commit, so it is committed here before any further measurement rather than after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cherry-pick of the profile-hook and assist-termination fixes. Resolved against this branch's cn1_globals: the merge kept the old entry-side hook in cn1BibopFastAllocNoZero alongside the new success-side one, which would have double-counted every fast-path allocation that falls back to codenameOneGcMalloc -- the exact error the change exists to remove. Needed here because the residual plaintext allocation still has an unattributed byte[] component, and cn1FusedLatin1Begin -- fused String plus byte[] payload -- was one of the entry points the profile never saw. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Under virtual threads the keep-alive loop waits for the next request by calling fill() directly, and it did so with parsedFromBuffer still set from the request it had just answered. fill() reads that flag as "midway through a request", so it took detachPreservingOffsets and copied the whole borrowed buffer -- on 99.5% of requests (1989689 detaches against 2000000 reads). The zero-copy read was working perfectly, every time, and handing the saving straight back one frame later. Clearing the flag once the response is on the wire lets fill() drop the borrow instead. Measured on /plaintext, one binary, the fix behind an env switch so the arms differ in nothing else, six interleaved pairs with the arm order rotating: ahead in 5 of 6, median +4.5%, and p99 better in 5 of 6. The allocation half is not statistical -- detach goes to exactly 0, removing one 97-byte array per request, which the corrected profile put at 37% of everything /plaintext allocates. Found with the allocation profile's new size histogram: 7073173 of 7073834 byte[] allocations were exactly 97 bytes, so one site rather than a dozen. Two earlier candidates had been eliminated by reading the code and both readings were wrong -- fill()'s fallback, which a path counter then showed never runs at all. The flag's real job is untouched. It guards the SECOND read within one request, where a body arrives after its headers and slices into the array are live; readRequest clears it on entry and raises it once the header block is parsed, so it covers exactly the window in which a Request exists. This point is outside that window by construction. Pipelining is also unaffected: the available() == 0 term still sends a buffer with bytes left in it down the copying path. BackendHttpIntegrationTest passes 21/21, including transactionRollsBack and authGuardsMutatingRoutes -- the two this guard broke when it was missing -- plus pipelinedRequestsAreNotLost, chunkedUpload and honoursExpectContinue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Compare-exchange rather than a plain store, so two threads allocating the profiled class at different sizes cannot merge their counts under one size. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Request and Response were the whole of what /plaintext still allocated once the borrowed-buffer copy went -- 80 and 88 bytes, one of each, every request -- so this is half of what was left. The route falls from 168.2 to 88.2 bytes per request and Request disappears from the allocation profile entirely; that half is exact rather than statistical. Throughput over thirteen interleaved pairs in one binary, arm order rotating, is a median +13.6%, ahead in 12 of 13, p99 better in 10 of 13. A profiled build shows +2.3% for the same change and that is not a contradiction: the profiler taxes every allocation, so that server is slower, allocates less per second, and the collector it is being spared matters less. Immutability is preserved where it was actually claimed. The class documents a Request as valid only for the duration of Handler.handle, and the array and slice table behind it were ALREADY reused; the fields simply stop being final so one object can be re-pointed. reset() runs while a request is being parsed -- after the previous handler returned and before the next is called -- so nothing mutates under a handler, which is what "immutable to its handler" meant. Every field is assigned with no unchanged case, headers most of all: it caches a Map built on demand, and carrying it over would answer one request's header lookups with another's. Nothing retains a Request past the handler: no field, no collection, no use after the call. BackendHttpIntegrationTest passes 21/21 including dtoRoundTrip, headersAndCookiesBind and pipelinedRequestsAreNotLost, which are the tests a stale field would show up in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The floor was a hand-edit to cn1_globals.m, which is the whole VM's file, kept as an uncommitted local change in one checkout -- which is why that file kept drifting from the branch it was supposed to match. The define is #ifndef-guarded precisely so a deployment can choose its own floor, so this belongs in the build that wants it. Measured on /plaintext at 64 connections: 4MB -> 30MB RSS, 8MB -> 49MB, 16MB -> 68MB, 24MB -> 98MB, with throughput and p99 flat across that sweep inside the run-to-run noise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # scripts/copyright-header-exclusions.txt # vm/ByteCodeTranslator/src/cn1_globals.h # vm/ByteCodeTranslator/src/cn1_globals.m
put(String) walks a string one charAt at a time. The response head's literals -- "HTTP/1.1 ", "\r\nContent-Type: ", "\r\nDate: ", "\r\nContent-Length: ", "\r\nConnection: keep-alive", "\r\n\r\n" -- are 82 of the ~97 characters a plaintext 200 emits, so the server was spending 51 million charAt calls a second at its own throughput to reproduce bytes that never change. fasthttp writes pre-encoded slices with copy. These are encoded once at class init and written through put(byte[]), which is a System.arraycopy, and status 200 gets its whole line as one constant so the number formatting goes too. Aimed by measurement rather than by reading the code. CPU accounting from /proc/<pid>/stat, which perturbs nothing, put CN1 at 1.36us of USER time per request against fasthttp's 0.74us while SYSTEM time was at parity, 1.62 against 1.51 -- so the I/O pipe was already equal and the whole gap was work we do ourselves. Two earlier candidates died on measurement first: strace showed nothing useful and distorted fasthttp by 27x, and the "we send Connection: keep-alive and Go does not" theory turned out to be worth six bytes, because fasthttp sends a Server header we do not. Measured, three interleaved pairs with the arm order rotating, one binary: throughput 566719 against 519456 req/s, +9.1% and ahead in all three; user CPU 1.36 -> 1.20us per request; system CPU unchanged at 1.6, which is the shape the diagnosis predicted. The remaining user-CPU gap to fasthttp is 0.46us from 0.62. Responses are byte-identical between the two paths, asserted by md5 over headers and body with only the Date line excluded, before any timing was taken. CN1_HTTP_FAST_HEADERS=0 restores the per-character path; both are complete and independent so the comparison stays runnable. BackendHttpIntegrationTest 21/21. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…locating Three changes that share one cause: the last per-request allocation, and the thread state that outlived every connection. FREEING THE THREAD STATE. markDeadThread only QUEUES a dying thread's TLD; cn1DrainDeadThreadPending migrates its pending allocations at the next mark and frees the TLD only if gcReleaseRequested is set. An OS thread gets that flag from its Thread object's finalizer, and cn1RetireVirtualThread -- the VM's own retirement path -- sets it explicitly because a virtual thread has no such object. The backend's freeImpl duplicates that path and did not, so every connection's TLD was queued, drained, un-flagged and abandoned: callStack arrays ~50KB, pendingHeapAllocations ~27KB, the try-block array ~15KB, all malloc'd. Measured at ~68KB per closed connection, resident memory 3MB to 65MB over 900 connections, and 249 collections returned none of it because every drain found the flag clear. DRAINING IT. That queue is only drained at mark start, so a server that churns connections while allocating almost nothing has no reason to collect and no other way to reclaim -- a sawtooth to 165MB over 2800 closed connections. Queued thread state is now its own demand signal. Allocation volume cannot express it: none of that memory was allocated by the mutator, so the counters the trigger watches never move. With both halves, resident memory is flat at ~34MB across 3200 closed connections where it previously reached 165MB. REQUEST.RESPOND. Response was the last per-request allocation on a route that allocates nothing else, and allocation drives both how often the collector runs and how much it holds. A handler can now ask the request for its connection's Response instead of building one. Valid for the duration of handle() and not beyond, which is the contract Request already carries and for the same reason; new Response(...) is unchanged for a handler that needs its own, and the HTTP/2 path, which has no connection to borrow from, still allocates. Verified before timing: pooled and allocating emit identical bytes across three requests on ONE connection, which is where a stale field would show and a single-request check would not. BackendHttpIntegrationTest 21/21 including pipelinedRequestsAreNotLost, dtoRoundTrip and headersAndCookiesBind; GcHeapIntegrity, BibopPageFloor, LowMemoryThrottle and LargeArrayGc 4/4 against the collector change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A CPU profile put java_util_concurrent_atomic_AtomicLong_incrementAndGet among the hottest symbols in the server, twice. requestsServed was incremented on every request, and with the host threads pinned to two cores each increment moves a cache line between them: one contended atomic in the hot path of a route that otherwise allocates nothing. Each host thread now counts into its own slot, spaced a cache line apart so two hosts never share one, and the health endpoint sums the stripes. The slot has a single writer -- the host that owns the connection -- so the increment is a plain add, and the index is resolved once per connection rather than per request. The reactor mode keeps the atomic: it has no hosts to stripe by. Measured, interleaved, one binary apart: 605832 against 582037 req/s (+4.1%), p50 97 against 101.5us, and user CPU 0.988 against 1.090us per request -- so the atomic was costing about 100ns of the 320ns that separated this server from fasthttp in user time. BackendHttpIntegrationTest 21/21 including metricsEndpoint, which reads the striped total. Worth recording how it was found: the candidate list going in was parsing, write barriers, safepoint handshakes and bounds checks. The barrier was measured at zero, and none of the others were this. The profiler named it in one run after several hours of ablation guessed at everything except a metrics counter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A profile of the plaintext benchmark put epoll_ctl at 4.65% of in-binary
self time. It was the ONESHOT re-arm: the kernel disarms a descriptor as it
delivers it, so every park cost an EPOLL_CTL_MOD to bring it back -- one
syscall per request, on a path whose dominant cost is already syscall
dispatch (musl's __syscall_cp_c, 19% of in-binary self time).
ONESHOT was guarding a hazard that only exists when several threads share a
poller, and virtual-thread mode has affinity instead: a descriptor lives in
exactly one host's epoll set and that host is not polling while it is inside
advance(). But it was also doing something the comment did not credit it
with. A virtual thread that answers RUNNABLE is queued in the ring, neither
running nor parked, and ONESHOT's disarm-on-delivery is what stopped epoll
reporting it again and having advance() resume a handle the ring was also
about to resume -- a use-after-free once the first resume finishes and frees
it. That invariant is now explicit: the RUNNABLE path removes the descriptor
and VtHost.armedByFd remembers it, so the syscall moves to the yield path
instead of every request.
Measured on two pinned cores, interleaved with rotating arm order, n=3:
new 650505 rps p50 73us p99 199us 2.40 us/req cpu
old 609484 rps p50 96us p99 162us 2.62 us/req cpu
fasthttp 608615 rps p50 88us p99 903us 2.32 us/req cpu
+6.7% throughput and a quarter off p50, with no overlap between the arms.
That puts the plaintext path at 1.069x fasthttp's throughput and 4.5x its
p99, at 3% more cpu per request.
BackendHttpIntegrationTest is 21/21, which exercises this: build.sh always
defines CN1_VIRTUAL_THREADS, so those run in virtual-thread mode over kqueue,
including shedsIdleConnections and the park/timeout path.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
String.charInternal was 4.92% of in-binary self time in a profile of the
plaintext benchmark, third behind syscall dispatch and serveOne. It came from
comparing raw buffer bytes against String constants one character at a time:
about eleven calls per request before a single header is read -- eight for the
version and three for the method -- and more for every header name matched.
The constants are now held as byte[] built once at class initialisation, so
the same comparison is a byte load.
The folded comparisons keep their constants ALREADY folded, which halves that
work again: only the bytes that arrived off the socket are passed through
foldAscii, instead of both sides on every character.
knownMethod was also doing the same walk twice. It asked for a folded match
AND an exact match, and an exact match implies the folded one, so the first
call could only ever agree with the second -- two passes over the method with
a foldAscii per character to answer what one pass answers.
Interleaved with rotating arm order, n=3, against the previous commit:
byte constants 659698 rps 2.371 us/req cpu
previous 652597 rps 2.393 us/req cpu
fasthttp 610101 rps 2.307 us/req cpu
Throughput ranges overlap at n=3 so the +1.1% is not separated, but the cpu
figure is lower in three reps of three, which is the effect the profile
predicted: charInternal was about 1.8% of total cpu.
Behaviour is unchanged where the folding matters. Ten probes against both
binaries -- upper, lower and mixed case Content-Length and Transfer-Encoding,
HTTP/1.0, HEAD, an unknown method and a lower case one -- answer identically,
including the 501 for "get", because HTTP methods stay case sensitive.
BackendHttpIntegrationTest is 21/21.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The diagnostic split in the /json handler asked a question and left it open:
is the distance to Go the byte writer, or the LinkedHashMap the handler builds
to hand to it? Measured on two pinned cores, 64 connections, interleaved with
rotating arm order, n=2:
generated DTO (2) 595158 rps 2.619 us/req
fasthttp 585906 rps 2.525 us/req
hoisted map (1) 547423 rps 2.849 us/req
map per request (0) 338167 rps 4.512 us/req
It is the container. The map costs 1.9 us of the 2.0 us that separated this
route from Go; hoisting it recovers most of that and the struct-shaped writer
recovers the rest, finishing slightly ahead of fasthttp. The byte writer does
not need porting to C -- at mode 2 it already serialises this object for less
cpu than Go spends on the equivalent.
Mode 0 stays the default: it is the honest cost of a handler that returns a
Map. An annotated DTO gets mode 2's shape from the processor already.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gcSatbCap only ever doubled. Nothing shrank it, and the take-side staging
buffer inside cn1SatbTake grew the same way, so a process that saw one busy
period kept both at the peak for its whole life. Measured on the backend, with
gcSatbTop read as 0 every time it was sampled -- none of it was in use:
plaintext satbCap 8MB of a 12MB RSS
/json DTO route satbCap 16MB
/json map route satbCap 8MB
Two thirds of the plaintext process was an empty write-barrier log. It is also
what made the footprint look unrelated to anything else: the run with the FEWEST
BiBOP pages had the MOST resident memory, because it was the one whose barrier
traffic had reached 16MB.
Trimmed in the sweep, beside the page trim, against the high-water batch since
the last trim rather than the instantaneous depth -- which is 0 there by
construction and would shrink to the floor every cycle and re-grow through
several reallocs on the next burst. The 4x slack and doubling target mean a
steady workload settles at a size it keeps.
Only shrinks when the log is idle: a non-empty log is live data the mark phase
has not taken yet. A failed realloc keeps the existing buffer, because realloc
is not required to succeed just because the block is getting smaller.
Interleaved with rotating arm order behind a calibration gate, n=3:
/json RSS 57.7MB -> 24.1MB 590056 rps against 587744
/plaintext RSS unchanged 647380 rps against 646246
Throughput is unchanged on both routes; the memory is 58% off the allocating
one. Plaintext does not move because response pooling runs one collection per
15s, so the trim almost never fires there.
GC gate: GcHeapIntegrity, GcOverflowSpiral, GcUncooperativeThread, LargeArrayGc,
BibopPageFloor and the 21 BackendHttpIntegrationTest cases all pass.
GcSteadyState's 768MB-ceiling scenario fails, and fails IDENTICALLY with this
change stashed (895.8s against 913.7s, same 600s timeout, same scenario) -- it
is the known local failure on this 16-core machine, not a regression.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The deferred-JSON path already avoided every copy on the body side: Json.write
serialises straight into the connection's reusable ByteSink, so no byte[] and no
String is ever materialised for the body. What it did not avoid was the Response
itself. Response.jsonValue is a static that allocates one per call, and that was
the ONLY thing the route allocated.
Profiled with -DCN1_GC_CONFORM over 10.9M requests on the DTO route:
88.1 bytes/request, and the histogram has one row that matters --
com.codename1.backend.HttpServer.Response bytes=961269408 count=10923516
count is one per request. The plaintext route had been pooled already and sat at
0.1 bytes/request. Request.respondJson puts the JSON route on the same footing:
the connection's pooled Response, reset and re-pointed at the value.
That takes the route to 0.1 bytes/request, and with nothing to collect the
collector stops running: ZERO cycles in a 15s run against 205. Which is the
whole point -- the collector shares the server's cores, so on this machine a
route that allocates pays for it in its tail, not in its allocator.
Production build, interleaved with rotating arm order, n=3:
rps p50 p99 cpu/req RSS gc cycles
pooled (3) 672158 72us 178us 2.33us 10.6MB 0
unpooled (2) 600949 89us 2843us 2.59us 22-27MB 205
fasthttp 592709 96us 1347us 2.51us 12.3MB -
1.134x fasthttp's throughput, 7.6x its p99, less cpu per request than it spends,
and a smaller resident set. For reference fasthttp is not allocation-free here
either: GODEBUG=gctrace=1 over 11.4M requests shows 60 collections, 3->3->0MB
each, about 16 bytes per request.
Mode 2 stays exactly as it was so the cost of the Response remains measurable
against mode 3, and mode 0 stays the default -- it is still the honest cost of a
handler that hands back a Map.
reset() already clears deferredJson and hasDeferredJson, so a pooled Response
reused for a plain body cannot carry a stale value into the next response.
BackendHttpIntegrationTest 21/21 and BackendDatabaseTest pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
scripts/check-copyright-headers.sh --base master reported it as the one file in the branch without a header. Same Codename One GPLv2 + Classpath Exception header as the demo beside it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
maven/backend/pom.xml was complete -- parent codenameone, sources from
${backend.dir}, a parparvm-sources classifier beside the compiled jar -- and
listed in no <modules> block, so nothing built it. The only reason it resolved
here was an install run by hand into a per-checkout repository months ago.
That matters because BackendPackageMojo resolves the artifact at run time:
resolve("com.codenameone", "codenameone-backend", ...)
so on a fresh clone, in CI, and in a release, cn1:backend-package and cn1:backend
would fail to find a runtime that the build never produced.
Placed after sqlite-jdbc, its only dependency. Verified by deleting the
hand-installed copy first and building the module from the reactor: all four
artifacts come out, including the parparvm-sources classifier the package goal
needs.
Note for the release: this now publishes codenameone-backend alongside the other
modules, which is the point -- an app cannot depend on a runtime that is not
published.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The branch adds a server-side runtime, two Maven goals and a published artifact, and the guide said nothing about any of it. This is the chapter a reader needs before deciding whether the thing is for them. It leads with what the backend does NOT replace, because that is the question a Java developer asks first and the wrong answer is expensive. Spring Boot, Jakarta EE, Quarkus and Micronaut carry a container, an ORM, a security stack and twenty years of operations; none of that is here and none is planned, and a team running a Spring service that works should keep running it. What the chapter argues instead is where the JVM's assumptions stop paying: cold starts charged per invocation, baseline memory charged for the life of an instance, sidecars and edge locations and short-lived processes. That is the region where Java is thin on the ground and Go and Node dominate, and where a Java shop ends up carrying a second language and a second copy of every model that crosses the boundary. The backend exists to remove that reason to leave Java, not to compete where the JVM already wins. The vertical-integration section is the other half of the argument: one annotated interface generates the app's asynchronous client and the server's synchronous half plus its dispatcher, so a contract change is a compile error rather than a response the app fails to parse in the field. Also covers the two goals and how they differ, the four packaging targets and why both libcs exist, the Lambda custom runtime, and a limits section that names the library ecosystem, the absent framework structure and the fact that throughput alone is not a reason to move. Placed in a new "Server side" part. Figures quoted are the ones measured here: about 8 MB static, a few milliseconds to first connection, about 3 MB idle. Gates: vale 0 issues across the guide, asciidoctor lint clean at --failure-level WARN, structure 123 documents, cross-references 1699 anchors, paragraph capitalization clean, snippets and links unchanged. The three vale-skip comments are documented exceptions where the contraction rule would attach a verb to the wrong word. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A generated project had no server side, so a developer who wanted one had to work out the module layout, the dependency and the two goals from the guide and write the pom by hand. The archetype now emits a backend module beside the platform ones. It is NOT built by default. The module sits in a profile activated by -Dcodename1.platform=backend, exactly like the platform modules, so a client-only app pays nothing for it and asking for it is explicit. The generated module deliberately does not depend on codenameone-core. A server has no display, and the compiler saying so at the import is more useful than a crash at start-up; the pom comment says this and points at the two ways to share types with the app, a module both depend on or a @restclient contract. BackendServer is a working handler rather than a stub: it installs the shutdown handler so a container stop drains in-flight requests, reads PORT and WORKERS from the environment, answers /healthz, and blocks in awaitTermination because the reactor threads are detached and a returning main would exit silently. Verified end to end rather than by inspection: installed the archetype, generated com.example.demo:mydemo from it, and compiled the generated backend module against codenameone-backend. The Java lands at backend/src/main/java/com/example/demo/BackendServer.java with the package substituted, the pom resolves to mydemo-backend with the right mainClass, and the root pom carries the profile. The first attempt failed generation outright: an XML comment cannot contain "--", and the wording had one. Worth knowing because the archetype reports it as an XmlPullParserException against a position in the generated file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A download had no server side, so the initializr answered the client half of a project and left the other half to be assembled by hand from the guide. The skeleton in common.zip now carries a backend module beside the platform ones, with a working handler rather than an empty directory. It is not built by default. The module sits in a profile activated by -Dcodename1.platform=backend, the same shape the platform modules use, so a client-only download pays nothing for it. GeneratorModel validates the new module's coordinates like the others but deliberately does NOT require a dependency on the generated common module -- and the test asserts that dependency is ABSENT. common is compiled against codenameone-core, a server has no display, and requiring it here would enforce exactly the mistake the module's own comment warns against. Verified against the real zip by mirroring what the generator does: apply the same content and path substitutions, normalize whitespace the way normalizedPom does, then assert the fragment validateModulePomCoordinates looks for. The coordinates resolve to <pkg>:<app>-backend:1.0-SNAPSHOT, the handler lands at backend/src/main/java/<pkg>/BackendServer.java with its package rewritten, and the root pom keeps balanced profile tags. The initializr's own suite could not be used for this: every test class in that module reports "Tests run: 0" locally, pre-existing and not specific to these sources, so the assertions added here get their first real run in CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 625401b152
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…under a read FileIo passed a path straight to open() and realpath(). A NUL inside it ends the path there, so "/etc/hosts" plus a NUL plus ".png" opens /etc/hosts while an application that validated the name it was given saw a .png and allowed it -- and validating an untrusted name by its extension and then opening it is the ordinary shape, not an unusual one. Measured before the fix: openRead answered descriptor 3 and realPath answered /private/etc/hosts for a path that names neither. The Java SE arm has always refused it, since Paths.get will not take a NUL, so the simulator proved the extension check worked and only the device opened the other file. Answered as "no such file" rather than thrown, because that is exactly what the Java SE twin answers and these methods report failure by return value. That is the third reachable NUL in this runtime after the sslrootcert path and the TLS verification name. The rule is the same every time: a string that crosses to a native is not the string the native reads. And close() freed the TLS session out from under a thread that was inside it. The handle IS the SSL*, SSL_read yields the VM thread while it waits, and DbPool.close() closes every connection it knows about -- including one a borrower is still reading from. The session is now freed by whoever leaves last rather than by whoever closes; close() still returns at once because it closes the DESCRIPTOR, which is what the blocked read is really waiting on, so the other thread comes back immediately and frees on its way out. Not argued from the code: the self-test's network arm, built without this fix, dies with SIGSEGV, and the crash report names ssl3_read_bytes under SSL_read under tlsReadImpl. With it the same arm passes five close-under-read rounds in three consecutive runs. The check is network-gated like the TLS checks beside it, so what CI gets from this is the FileIo half; the TLS half is a local gate and is documented as one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3e6c2bf9a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…re read in bind() passed its host straight to a native. A NUL ends the name there, so "127.0.0.1" plus a NUL plus ".example" binds 127.0.0.1 -- and "0.0.0.0" spelled that way binds EVERY interface on the host -- while whoever approved the configured name saw an .example one. Measured before the fix, the check answers "bound". The Java SE arm fails such a name at resolution, so the two arms disagreed about which interfaces a configured host means and the one that binds wider is the packaged one. Null still means every interface: that is this method's own way of saying so, and the control checks it still does. That is the fourth reachable NUL here, after the sslrootcert path, the TLS verification name and the file path. I am no longer claiming the class is closed; every string that crosses to a native is a candidate until it is checked. A response was also read past its own end. Content-Length is the whole of the framing, so what follows the declared bytes is not part of the message, and handing it back meant a caller acted on the peer's answer with something else appended -- a second response, or bytes an attacker put after a short one. Measured: a body declared as 2 came back as "hi" followed by twenty-nine bytes nobody asked for. A short body stays a transport failure and is still reported as one; a long one is the peer contradicting its own framing while the framed answer is perfectly readable, so this keeps exactly what was declared. And backend-package recompiled the module's sources as UTF-8 whatever the module said. These are the SAME sources the lifecycle has already compiled, so reading them differently is a second, disagreeing compilation of one tree: a module declaring another encoding either fails packaging on bytes javac accepted a phase earlier or -- worse, because nothing says so -- translates string literals that are not the ones the JVM build produced. Resolved the way the compiler plugin resolves it: an explicit <encoding>, then project.build.sourceEncoding, and UTF-8 only when the module says nothing. The platform default is deliberately not the last resort, since it makes a build depend on the machine that runs it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a28536ef90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…xact match Db.open passed its path to sqlite3_open, which reads it as a C string: a NUL inside it ends the path there, so "allowed.db" followed by a NUL and "/ignored" opens allowed.db while a caller that checked the directory or the suffix it was handed approved something else. Measured before the fix: the check reports "opened". Reported as "could not open", which is what this method already says for a path that names nothing. That is the fifth reachable one, after the sslrootcert path, the TLS verification name, the file path and the bind address. The If-Range finding beside it is REFUSED, and this records why where it will be read. The review asked for the comparison to become "the file is no later than this date", reasoning that a file older than the client's validator cannot have changed since it. That is the rule for If-Unmodified-Since, and RFC 7233 3.2 singles out the difference in as many words: "this comparison by exact match, including when the validator is an HTTP-date, differs from the 'earlier than or equal to' comparison used when evaluating an If-Unmodified-Since conditional". If-Range takes the strong comparison because of what it authorises: splicing new bytes onto a prefix the client already holds. A file whose timestamp went BACKWARDS -- restored from a backup, rolled back, or served from a replica whose clock differs -- is a different representation that happens to be older, and answering 206 for it staples two representations together and calls the result a complete download. Sending the whole file costs one transfer; the splice costs a corrupt file the client believes in. Pinned rather than only argued: the check drives a request whose If-Range is seventy years later than the file and requires 200, and a control with the validator the client was actually given requires 206. Applying the change the review asked for turns the first into 206, which is the test failing, which is the point of writing it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6656401cf2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…instance Six of these have been reported one at a time and I have twice said the class was closed, so this one is answered with the whole surface instead. Every native in the packaged runtime that takes a String: Web (method, url), FileIo (root, relative, path), Http2 (status), Tcp (host, caFile), ServerSocket (host), Db (path, sql, bound value), Tls (certPath, keyPath). Classified rather than assumed. Http2's status is String.valueOf(int) and cannot hold a NUL. Db's bound VALUE already crosses with its encoded length, which the native documents. Web, Tcp, ServerSocket, FileIo and Db's path are checked already. That left two. The TLS certificate and key paths were unchecked, and what they name is the identity the whole server presents and the private key behind it. Measured before the fix, OpenSSL was handed "/tmp/cn1-cert" for a string that reads "/tmp/cn1-cert\0.pem" -- it failed only because nothing was at the truncated path, which is the attacker's choice to make, not ours. And the statement text. sqlite3_prepare_v2 is called with -1 deliberately, and the native's note said a truncated statement is incomplete and gets rejected. That is true of most cuts and not of all: "... WHERE id='1'" followed by a NUL and " AND owner='bob'" truncates to a complete statement whose authorization clause has gone, and the self-test runs exactly that DELETE -- before the fix it reports "accepted". The note now says what is actually true, because the Java side refuses the statement. The sweep also turned up a divergence rather than just an omission: JDBC is length-aware, so the SAME statement RAN on the Java SE arm. The development loop was accepting what the packaged build silently rewrites, which is the worse half of a divergence, so that arm refuses it too. The Java SE runtime has no TLS at all, so the two path checks are the translated arm's and the check says so with a NOTE rather than passing quietly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92fef35188
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…dshake is discarded The truncating-string class has a second half I did not enumerate. The last sweep covered every NATIVE taking a String; this one is on the WIRE. Both database protocols end each startup field with a NUL, so a NUL inside a value does not truncate it -- it ENDS that field and the rest becomes the next one. A database name of "app", a NUL, "application_name", a NUL and "allowed.tenant" selects the app database and sets a startup parameter nobody asked for, while the whole string still passes a suffix check on the name the application thought it was connecting to. A URL carrying %00 decodes to exactly that. Guarded in writeCString rather than at the two fields the review named: that is the one place every such field is written on each protocol, so a field added later is covered without anyone remembering. Demonstrated rather than argued -- without it the PostgreSQL client is caught in a stack that has already written the packet and is waiting in authenticate/readMessage for the reply, and the MySQL one connects successfully with the smuggled parameters sent. The other half is a defect in this branch's own earlier fix. close() now coordinates with in-flight TLS reads and writes, but the UPGRADE holds no claim -- there is no session to claim until the handshake returns. A close during it closes the descriptor, which is what makes the handshake fail; the race left is the narrow one where it had just succeeded, and publishing then reported success on a closed socket and left an SSL* only a second close would free, by which time its descriptor number may belong to another connection and SSL_shutdown would write TLS bytes into that one. startTls now rereads the connection under the monitor before publishing, and discards the session when closure won. Discarding needs a free that does NOT touch the descriptor, so tlsDiscardImpl is SSL_free alone -- there is nothing to tell the peer when the socket it was talking on is gone. Both TLS builds define it, and the build was run with CN1_NATIVE_VERIFY=strict so the symbol name is checked rather than assumed. The race itself has no deterministic test: it is a window between a native returning and a field being assigned. What is verified is the symbol, the non-racing path, and that the suite still passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two files both sides had touched. GeneratorModel merged cleanly and was checked rather than assumed: master's rewritten launcher instructions and this branch's backend coordinate validation are both present. common.zip is binary, so git could only report the conflict. The two changes are disjoint -- master's #5803 updated the launcher entries (run.sh, run.bat, build.sh, build.bat, mvnw.cmd) and this branch added the backend module template and a profile to the root pom -- so the resolution takes this branch's archive and updates those five entries from master's. Verified by listing all three archives: the result differs from master's only by the backend entries and the root pom, the five launchers are byte-identical to master's, and run.sh and build.sh keep the 0755 the others do not have. Master's four deleted files are deletions, not losses: #5786 generates the properties binding figure and its snippets from the sample now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e8d9a3f9f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81c0c9566e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
stop() had no claim on the shutdown. Two overlapping paths -- a signal handler and the application's own cleanup is the ordinary pair -- both snapshotted the same live descriptors and ran the raw close that bypasses drop()'s ownership check, so the first close releases the number and the second closes whatever has since been given it. Both also reached the unsynchronized Tls.close() and freed one native context twice. It is claimed with a CAS now, and the loser returns rather than waiting: its intent is already being carried out, and blocking it would deadlock a handler that calls stop() against the drain that is waiting for that same handler. That handler is the other half. Its request has already incremented the counters stop() drains and cannot release them until stop() returns, so the wait ran the whole window out for something the calling thread was itself holding -- and the sweep afterwards closed the very descriptor the reply was owed on. An admin endpoint that shut the server down answered its caller with a dropped connection. The drain now discounts what the calling handler holds and the sweep leaves that one descriptor alone, so the response still goes out. Knowing which descriptor that is needs per-request state, and the ThreadLocal it uses was measured rather than assumed: on this VM it really is per VIRTUAL thread, not per host. Two connections interleaved on one host each read back their own value and neither saw the other's -- worth stating because everything else in this file that is per host is per host precisely because the virtual threads share it. The A/B is unusually blunt. Before the fix the suite does not fail, it DIES: three runs out of three, an uncaught "Socket write failed" several checks later in oneLateBodyRequest, which is the second close landing on a descriptor that a later connection had been given. The check says so in its own comment, because that is where the next person will be looking. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 021748a493
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 021748a493
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…till waits dechunk returned as soon as the zero-size line arrived, so a response that stopped right after "0" was handed back as a complete body and whatever bytes happened to follow were accepted as a trailer nobody validated. RFC 9112 7.1 ends the message with the last chunk, the trailer section, and a final CRLF; all three are required now. The trailers are still dropped -- this client has no caller that reads them -- but they have to BE header fields and the section has to be terminated, because a truncated response is the one thing a client must not round up to success. The second change reworks this branch's own stop() claim from the commit before it. That version let the losing caller return at once, which quietly changed a contract: stop() used to return only once the server was down, and a caller that carried on early would do so while the virtual-thread slot was still held and descriptors were still open. The loser now waits for the winner, bounded by the whole teardown the winner is allowed, so a handler calling stop() still cannot wedge itself against a drain waiting for that same handler. WHAT I HAVE NOT EXPLAINED: one run of the self-test, before this change, hung nineteen minutes inside stop() under negativeConnectTimeoutsAreRefused. It has not recurred in fourteen runs since, and the early return above is the only mechanism I could identify that would let one server's teardown overlap the next server's start, so it is gone. That is a removed mechanism and a failure to reproduce, NOT a root cause, and it is written here rather than left out so the next person who sees a shutdown hang starts with this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… handler's headers Content-Length was read as "the first one, and unreadable means absent", so the framing depended on field ORDER. Measured: "Content-Length: 5" before "Content-Length: 10" with ten bytes came back as "hello", and the SAME response with the fields reversed came back as "helloworld" -- one response, two bodies, chosen by which field the parser happened to reach first. An unparseable value turned the truncation check off entirely, and a negative one passed straight through it. Every occurrence is read now and they have to agree, which RFC 9112 6.3 makes a requirement; repeating the same value is still one length, since that is the duplicate the spec lets a recipient collapse. The HTTP/2 refusals carried the handler's own headers. When the body budget or the descriptor ceiling refuses a response, the 503 that replaces it was submitted with that response's header list -- so a login handler's Set-Cookie went out on a 503 saying the operation was unavailable, and the client is told the request failed and is authenticated anyway. Validators and cache directives went out the same way, describing a body that was never sent. All four refusal sites build their own header list now, which is empty: what the server says on its own behalf is the status. And the caller-aware shutdown from earlier in this branch missed the HTTP/2 counter. serveHttp2 holds a turn for as long as the handler runs, exactly as the request and the connection are held, so an h2 handler calling stop() was still waiting on a counter that was its own. The turn is discounted with the rest now, and awaitTermination no longer returns on the flag alone -- fullyStopped is reachable with a handler still inside a request, and main returning there takes the detached threads and that last response with it. The two HTTP/2 behaviours are argued from the code and NOT measured: proving them needs the harness to decode an HPACK response header block, which it cannot do -- its h2 client reads a status and nothing else. The Content-Length half is measured, both orders, with the single and the repeated-identical cases as controls. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AWS_CONTAINER_CREDENTIALS_FULL_URI was used exactly as the environment gave it. Pointed at an http:// host that is not this container's, the request carries the container authorization token TO that host and brings the role's access key, secret and session token back from it, in the clear, for anything on the path. A deployment typo reaches that, and so does one injected environment value. The rule is the one the AWS SDKs apply: https to anywhere, http only to this host's loopback or to the ECS and EKS link-local addresses, which ARE the credential service. The loopback test parses the address instead of matching text, because the obvious version of this check is itself a hole: "127.evil.example" starts with the loopback digits and is a NAME, and a name resolves wherever its owner says. Same for "169.254.170.2.evil.example", and for a userinfo that spells a loopback address before the @ -- all three are in the check, and all three are refused only because the host is taken from the right part of the URL and read as four decimal octets. Driving this needed a seam rather than an environment variable: fromContainer() reads the URI from the environment, which a running process cannot set for itself, so a public-path test could put one value per process run to it. CredentialEndpointProbe lives in the runtime's package inside the demo tree -- the arrangement FileCountProbe already uses here -- so every case is exercised without widening the runtime's API for a test. Ten cases, and with the rule neutered exactly the five refusals fail while the five allowances still pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…l inside The TLS session got this treatment earlier in this branch; the descriptor under it did not. close() closed the number while another thread could be entering read() or write() with it, and a closed number is handed to the next socket this process opens -- so the read is served, silently, by an unrelated connection. A blocked write has the same shape: its native retry loop can issue another send() after the close. The claim now covers the whole connection rather than the session alone. read() and write() take the descriptor and the session together under the monitor, so they describe one instant, and close() releases neither while anyone holds them. What close() does instead is SHUT THE SOCKET DOWN, which breaks the connection without giving the number back; the last operation out closes it for real and discards the session. Nothing in flight is still the plain teardown it always was. close() returns at once either way, as it must: the thread it is cancelling may be one that never comes back. The cost of that is a descriptor held until such a thread returns, which is a leak in a case that was previously a wrong answer. What the new check covers is the half my own change could break: shutdown has to wake a blocked reader as reliably as close did, or cancelling a read would hang instead of returning. Three runs, and it returns in well under the peer's silence. The reuse half is by construction -- the number is not released while anyone is inside a native with it -- and is not observable from Java, which the check says rather than implies. Built with CN1_NATIVE_VERIFY=strict so the new shutdownImpl symbol is checked rather than assumed: a wrong name there compiles, links, and silently drops the method. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A contract declaring @get("/café") generated a route that could never run. The dispatcher compares against a target that arrived as OCTETS, one char per byte, so the literal written in source as one character is a different string from the two the request carries. Nothing failed; the route simply never matched, which is the shape of defect this file exists to catch at build time. REFUSED rather than encoded, which is not what the review asked for. Encoding the literal to UTF-8 octets does fix the raw spelling -- and leaves the other one: canonical() resolves percent-encoding only for unreserved ASCII, so "/caf%C3%A9" would still miss. That spelling is what a browser sends, and it is what this contract's own generated CLIENT half would produce, so the fix would leave the two halves of one contract disagreeing about its route -- with the disagreement now depending on which client asked. Refusing says so once, to the person who can change it. It is also the rule the @RestController generator has always applied, in byteArrayLiteral, in the same words and for the same reason: "a route pattern is written in source, and a non-ASCII one would have to be compared against its percent-encoded form on the wire". The two halves of the system now agree about that as well. With the check removed the new test reports an empty error list -- the route generated, silently -- and the ASCII control passes either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04945ea7f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…text The first of these is a regression from earlier in this branch. Making dechunk require its terminator was right for a truncated body and wrong for a response that has none: a HEAD may carry "Transfer-Encoding: chunked" to describe how the GET would have been framed, and a 304 may carry the same metadata, while both correctly end at the blank line. Handing the decoder those zero bytes made it report a complete response as truncated -- measured, both cases, "Truncated chunked response: no chunk size line". The RFC 9110 6.4.1 rule that already stops the length check now stops the transfer decoding with it. A percent-escape in a database URL can also be individually valid and collectively meaningless: "%C3%28" is a lead byte followed by something that cannot continue it, and new String does not refuse that, it substitutes U+FFFD. The password, database name or CA path silently became a different value, and the operator saw a remote authentication failure rather than a malformed setting. Measured before the fix: the URL sails past and the client goes on to CONNECT, which against a real server is an attempt with the wrong credential. The third finding is REFUSED and the reason is recorded where the code is. It asked for a GC keepalive around the blocking recv(), since after the yield only the C locals refer to the Java array. The shape is right and this VM does not have the gap: cn1_globals.h declares conservative native-stack scanning DEFAULT ON -- in its own words, for methods whose "object roots live in native C locals ... rather than threadObjectStack" -- and CN1_YIELD_THREAD captures this frame as it parks. The build that turns it off is a documented GC ablation arm that also requires the frameless codegen to be turned off, which puts the roots back on the object stack; both halves move together. The iOS port's hand-written keepalive is a different runtime with different roots. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7eb9b98b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…s no plugin %2F and %2f are the same octet, and RFC 3986 6.2.2.1 normalises the digits to upper case for exactly that reason. A retained escape was left as it arrived, so a mount or a literal route declared with one spelling was missed by the other -- and a request that changed nothing but the case of a hex digit fell past a protected literal into whatever dynamic route followed it. Measured: a mount declared /a%2Fb answers "not ours" to /a%2fb, and declaring it the other way round fails the other way round. Fixed in all FOUR canonicalisers, not the one the finding named: the request side, the declared-path side beside it, the processors' shared pattern canonicaliser, and the canonical() the dispatcher generates. Any one of them left alone would put the two halves of a contract back into disagreement, which is the same shape as the encoded-literal defect earlier in this branch. Separately, a MySQL greeting that advertises CLIENT_SECURE_CONNECTION without CLIENT_PLUGIN_AUTH -- what the older compatible servers send -- ends its scramble with a NUL and names no plugin after it. Reading whatever was left found that terminator and took it for a plugin name, an empty one, which nothing implements: measured, the connection is refused with "Unsupported MySQL authentication plugin ''" instead of proceeding with the mysql_native_password the variable already held. The name is only read when the capability says one was sent, and the stub greeting now has both shapes so each is a control for the other. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92ff34f762
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…body is not invited A request may end with a trailer section, and those fields were appended to the same array Request.getHeader reads. A proxy that authenticates by setting a header in the INITIAL block -- and stripping whatever the client sent -- is then defeated by a client that puts the same name after the body instead, and the handler cannot tell the two apart. The HTTP/1.1 path consumes its trailer section and discards it, so this was also the two protocols disagreeing about what a header is. Only the initial block is kept now, which is nghttp2's own distinction (NGHTTP2_HCAT_REQUEST) rather than a guess about frame order. The second is the HTTP/1.1 100-continue fix from earlier in this branch, applied to the protocol that still had the hole: a declared content-length past CN1_H2_MAX_BODY_BYTES was answered with 100, and the reader then resets the stream the moment the body crosses that same ceiling. The client uploads megabytes to have them thrown away. The declared length is read before the expectation is answered now, and a final 413 replaces the invitation. Both are measured on the wire. /headerprobe answers 409 when a named field reached the handler and 200 when it did not, so a status-only client can carry the verdict, and the test sends the same field twice -- once in the initial block, once as a trailer -- so each is the other's control. Without the fixes: the trailer case answers 409, and the oversized expectation answers [100]. The h2 client had to learn to send a trailer section for this, which is worth having: nothing else in the harness exercised HEADERS after DATA. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74953f80eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
readResponse ran to EOF into a growing buffer and looked at no framing until the connection closed, so a peer that never stops sending was a peer this process grew a buffer for until it could not. The endpoint does not have to be hostile -- a misconfigured one that streams is enough -- and the failure lands on the whole process rather than on the request that caused it. Measured, and it is worse than the heap: with the bound removed the self-test dies with ArrayIndexOutOfBoundsException: -1 out of ByteArrayOutputStream.write, which is the capacity computation overflowing on the way up. Not an OutOfMemory that a deployment might at least recognise -- an index error from inside a growing buffer. The sibling client already had this bound: libcurl's write callback stops the transfer past CN1_WEB_MAX_RESPONSE_MB, default 64. This is the same ceiling with the name its own family uses, CN1_HTTP_MAX_RESPONSE_MB, and the same 1..2047 range, so the two clients answer alike rather than one of them being the way in. The check runs at the default rather than a special-cased small bound, which costs the suite a few seconds of loopback and tests the ceiling deployments actually get. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02848357be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+142
to
+143
| return new S3(Credentials.resolve(), resolved, | ||
| "s3." + resolved + ".amazonaws.com", false, true, true); |
There was a problem hiding this comment.
When resolved is cn-north-1 or cn-northwest-1, this constructs a commercial-partition hostname ending in amazonaws.com, while AWS China S3 endpoints end in amazonaws.com.cn. Consequently, every request and presigned URL created through forRegion() targets the wrong hostname for deployments in the China partition; select the endpoint suffix according to the region's AWS partition.
Useful? React with 👍 / 👎.
| channel.configureBlocking(false); | ||
| selector = Selector.open(); | ||
| channel.register(selector, SelectionKey.OP_READ); | ||
| return selector.select(timeoutMillis) > 0; |
There was a problem hiding this comment.
When timeoutMillis is zero—the translated implementation's documented pure-probe mode—Selector.select(0) waits indefinitely rather than returning immediately. A caller using awaitReadable(fd, 0) on the Java SE twin can therefore hang until bytes arrive, whereas the packaged implementation immediately returns false; use selectNow() for the zero-timeout case.
Useful? React with 👍 / 👎.
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
3 participants
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.
Adds a server-side runtime that runs a Codename One handler through the ParparVM
pipeline: Java or Kotlin translated to C and compiled into one static native
executable with no JVM under it. About 8 MB, a few milliseconds to first
connection, about 3 MB idle.
What this is for, and what it is not
It does not replace Spring Boot, Jakarta EE, Quarkus or Micronaut, and it is not
trying to. Those carry a container, an ORM, a security stack and twenty years of
operations; none of that is here or planned.
It targets the region where the JVM's assumptions stop paying: cold starts
charged per invocation, baseline memory charged for an instance's life, sidecars,
edge locations, short-lived processes. That is where Java is thin and Go and
Node dominate, and where a Java shop ends up carrying a second language and a
second copy of every model that crosses the boundary. Either as a piece of a
larger deployment or as the whole server for a small project.
The vertical integration is the other half: one
@RestClientinterface generatesthe app's asynchronous client and the backend's synchronous half plus its
dispatcher, so a contract change is a compile error rather than a response the
app fails to parse in the field.
Where it stands against Go
vm/backend/benchmarksholds the harness. Two pinned cores, 64 connections,interleaved with rotating arm order, against fasthttp:
The /json figures are the generated-DTO path answering off a pooled response.
A handler that returns a
LinkedHashMapper request is about 0.58x, which thebenchmark keeps as its default because that is the honest cost of that shape.
Notable changes outside vm/backend
cn1_globals.mgainscn1SatbTrim. The SATB write-barrier log and its stagingbuffer only ever doubled and were never given back, so a process that saw one
busy period kept the peak for life -- 8 MB of a 12 MB plaintext process was an
empty buffer. Trimmed in the sweep against the recent high-water mark. This
reaches every Codename One target, not just the backend.
maven/pom.xmlbuildsmaven/backend, which was in no<modules>block, sonothing built the artifact
BackendPackageMojoresolves at run time.cn1:backendandcn1:backend-package, and the@RestClientserver-half processor.
backendmodule, behind-Dcodename1.platform=backendso a client-only app pays nothing for it.Testing
BackendHttpIntegrationTest21/21, plus the database and JavaSE-runtime suites.GcHeapIntegrity,GcOverflowSpiral,GcUncooperativeThread,LargeArrayGc,BibopPageFloor.GcSteadyState's 768 MB ceiling scenario fails on the dev machine and failsidentically with the SATB change stashed (895.8s against 913.7s, same timeout,
same scenario), so it is the known local failure rather than a regression. It
is
@Tag("benchmark")and runs in the benchmark job.--failure-level WARN,structure, cross-references, snippets, links, paragraph capitalization.
codenameone-maven-plugin: 0 findings. Copyright, controlcharacters and cast-semantics gates clean over the branch.
backend module compiled against
codenameone-backend.PMD and Checkstyle were not run locally; CI is the first run for those.