Async liveness: bounded write parks, a classified accept loop, resolv.conf search/ndots - #7
Open
blacktrs wants to merge 10 commits into
Open
Async liveness: bounded write parks, a classified accept loop, resolv.conf search/ndots#7blacktrs wants to merge 10 commits into
blacktrs wants to merge 10 commits into
Conversation
Every write site took the unbounded writable slot, so a peer that stops reading — a zero TCP window, the reverse-slowloris — wedged the writing fiber forever: the task never settled, its scope never closed, the fd was never released, and nothing said why. stream_set_timeout() is the STREAM's timeout in php, not the read's, so it now sets $wtimeoutMs alongside $rtimeoutMs; __mc_wait_write mirrors __mc_wait_read (writableFor, 60 s floor, $timedOut on expiry) and bounds __mc_stream_sendv, fwrite and the raw Async\\read/write parks. Expiry is a short write, never an exception. The TLS WANT_READ/WANT_WRITE loop in __mc_stream_fill gets an ABSOLUTE deadline: both directions parked unbounded and the loop can alternate between them, so a per-park budget would let a half-record peer live twice-forever with timed_out false. Async\\accept stays unbounded on purpose — an idle listener is not a stalled peer.
The accept loop re-parked on ANY failure. Out of descriptors accept(2)
fails while the pending connection STAYS QUEUED, so a level-triggered
listener stays readable: the park returns at once, accept fails again, and
the loop spins without ever suspending — starving every sibling task. It
does not even trip the watchdog (it suspends each iteration); it shows only
as an exploding wakes counter.
__mc_accept_class splits the failure four ways — park / retry now
(EINTR, ECONNABORTED, EPROTO: no readiness edge is coming for a peer that
already left) / back off (EMFILE, ENFILE, ENOBUFS, ENOMEM) / fatal — and
stream_socket_accept folds its bounded and unbounded branches into one loop
over an absolute deadline. Overload SLEEPS rather than re-arming readiness,
since readiness carries no information there.
Kept separate from __mc_sock_wouldblock and of the opposite polarity: there
an unknown errno parks, here it is fatal. socket_accept gets the immediate
retry only — php reports EMFILE and returns false, and ext/sockets leaves
the serving policy to the caller. No exception at either site: php's
documented contract is a false return, and throwing would break every
while(true){accept();} loop.
Only nameserver lines were read, so a short name had no way to be qualified: the async resolver could not answer it and the lookup fell through to the BLOCKING getaddrinfo walk, stalling the loop. Inside compose and kubernetes short names are the norm — invisible on a laptop, unavoidable in a container, in exactly the deployment async exists for. Every parser takes TEXT (__mc_resolv_text is the only reader), so the whole of resolv.conf handling is offline-testable — the AOT runner injects no env. __mc_dns_candidates applies the ndots rule; __mc_resolve_async walks the candidates and caches under the ORIGINAL name, and stops only when nobody answered (a timeout must not be multiplied by the suffix count). resolv.conf is parsed once per lookup, down from twice. Two compiler bugs surfaced doing it: - a static local inside a NAMESPACED function emitted @ns\fn__sl_x — a backslash in an unquoted LLVM identifier, which does not parse. Every static local in the tree happened to sit in a global-namespace file, so the whole class of function was unbuildable and nobody knew. - USE-AFTER-FREE: a string ternary's borrowed arm got no retain. $out = $out === '' ? $s : ($out . ',' . $s) stored $s's buffer borrowed; the next iteration freed it, the allocator handed the same block back, and the accumulated value silently became the newest element repeated. A string-typed conditional is now an owned producer like a concat — arms normalised to +1, consumers uniform (fresh temp / owned local / no co-owner retain).
Semaphore and mapConcurrent were documented, but the example a reader copies spawned per accept with no ceiling — so the first server written from it gets unbounded concurrency and an OOM under load instead of a queue. The permit is taken BEFORE the accept, so at the ceiling the queue stays in the kernel backlog; it is released in the CHILD's finally, since the parent's runs once. http_transparent stays unbounded on purpose (it exists to produce a wrk number) and now says so. The doc also records why there is no built-in per-scope task ceiling: spawn() is the wrong place to fail — the exception lands in the acceptor, the one task that must survive an overload.
The first cut also taught the consumers that a string-typed conditional is an owned producer (fresh temp / owned local / no co-owner retain). That over-released: an arm which is not itself string-TYPED — a cell or unknown carrier that coerces to the string — gets no retain here, so the release side freed what nobody owned, and preg_full and socket_sendmsg read the freed bytes. Keep only the half that cannot corrupt: arms get their +1, consumers keep treating the result as borrowed. Retaining without releasing leaks a string (the concat arm already did); releasing without retaining corrupts one.
The async runtime is the largest thing here that php cannot run, and the README did not mention it at all. docs/superset.md now catalogues the whole surface with no oracle — concurrency, compile-time attributes, FFI, modules, types, memory — together with the rules it lives under: hand-written expected output, inert under Zend wherever possible, zero cost when unused, no php.ini. docs/async.md gains what this epic actually changed (bounded writes, the classified accept loop, search/ndots) and loses the two entries that are now done. The fiber-stack ceiling moves into 'Not yet' as an unmeasured number rather than an assumption. Also fixes the difftest DIFF the new dns case introduced: it echoed a literal BEFORE its first manticore-only call, so php produced stdout and difftest reclassified a PHP-SKIP as a real diff.
The status wall opened with numbers that go stale the moment a case lands (467/467, 458/0, ~228 functions, 5x2) and the install story was three lines in the middle of it. Requirements and install now come FIRST, with the toolchain floors, the per-OS packages and the -dev caveat that actually bites; then a Working-with-it section, then the language surface. Benchmarks condense from three tables to one representative slice plus the headline numbers — bench/run.sh is the source of truth for the rest. Gate counts are gone; the gate COMMANDS stay, which is what a reader can act on. Limitations lose the resolved multi-object-linking entry and gain the .sig class-export gap and blocking file I/O, both of which a user meets in practice.
…l fix The rcode holder sat in Dns.php, the one namespaced stdlib file, so building it needed the sanitizeSym fix that shipped in the same commit. A compiler generation WITHOUT that fix therefore could not build the tree at all: bin/build died on lib/manticore_stdlib.o and only a cold seed recovered. The stability gate found it the honest way — five self-front-end rebuilds in a row, all dead on the same symbol. The holder moves next to __mc_net_errno, whose idiom it copies anyway, in a global-namespace file. The compiler fix stays — it is a real bug for user code, and static_local_namespaced now pins it with php as the oracle (a namespaced function, a namespaced method, and the once-only initialiser). A self-hosting compiler must not need itself-plus-one-fix to build.
manticore_self.ll is 954k lines and was neither tracked nor ignored, so the first git add -A after a gate run committed it.
Gate numbers in a status doc are wrong the moment a case lands — the gate COMMANDS are what a reader can act on. In their place, the two things that actually cost time: bin/build green says nothing about tools/selfhost.sh (library-with-flattened-namespace vs one module), and never ship a compiler fix together with tree code that needs it.
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.
Closes items 1–4 of the async liveness review. The suite said the runtime was
correct; none of it said what happens when a real server is put under load.
Each item below had a file:line but no reproduction — writing the repro was
step one, and two of them only became visible once written.
1. A write park had no deadline
Every write site took the unbounded writable slot, so a peer that stops reading —
a zero TCP window, the reverse-slowloris — wedged the writing fiber forever:
the task never settled, its scope never closed, the fd was never released, and
nothing said why, because from the scheduler's side that task was legitimately
waiting on I/O.
stream_set_timeout()is the STREAM's timeout in php, not the read's, so it nowsets
$wtimeoutMsalongside$rtimeoutMs;__mc_wait_writemirrors__mc_wait_read(the existingwritableForhook, a 60 s floor,$timedOutonexpiry) and bounds
__mc_stream_sendv,fwriteand the rawAsync\read/writeparks. Expiry is a short write, never an exception.
The TLS
WANT_READ/WANT_WRITEloop gets an absolute deadline: bothdirections parked unbounded and the loop can alternate between them, so a
per-park budget would let a half-record peer live twice-forever with
timed_outfalse.
Async\acceptstays unbounded on purpose — an idle listener is not astalled peer.
2. The accept loop ignored errno
Out of descriptors,
accept(2)fails while the pending connection staysqueued, so a level-triggered listener stays readable: the park returns at once,
accept fails again, and the loop spins without ever suspending — starving every
other task. It does not even trip the watchdog (it suspends each iteration); it
shows only as an exploding
wakescounter.__mc_accept_classsplits the failure four ways — park / retry now (EINTR,ECONNABORTED,EPROTO: no readiness edge is coming for a peer that alreadyleft) / back off on a timer (
EMFILE,ENFILE,ENOBUFS,ENOMEM) / fatal —and
stream_socket_acceptfolds its bounded and unbounded branches into one loopover an absolute deadline. The overload case sleeps rather than re-arming
readiness, since readiness carries no information there.
Kept separate from
__mc_sock_wouldblockand of the opposite polarity (there anunknown errno parks, here it is fatal). No exception at this site: php documents a
falsereturn, and throwing would break difftest and everywhile (true) { accept(); }.3. Unqualified hostnames fell back to the blocking resolver
Only
nameserverlines were read, so a short name had no way to be qualified andthe lookup fell through to the blocking
getaddrinfowalk — stalling the wholeloop, in exactly the deployment async exists for (
db,redis,service.namespaceare the norm in compose and kubernetes, which is why this wasinvisible on a laptop).
Every parser takes text, so the whole of resolv.conf handling is offline
testable — the AOT runner injects no env. The cache key stays the original name,
and the candidate walk stops only when nobody answered, so one dead resolver is
not multiplied by the suffix count.
4. Backpressure is now in the idiom
A
Semaphoreinexamples/async/server.php, taken before the accept andreleased in the child's
finally, so at the ceiling the queue stays in the kernelbacklog.
http_transparent.phpstays unbounded and says why (it exists to producea
wrknumber). The doc records the decision not to add a per-scope taskceiling:
spawn()is the wrong place to fail — the exception would land in theacceptor, the one task that has to survive an overload.
Four general bugs found on the way
staticlocal in a namespaced function emitted@Ns\fn__sl_x— abackslash in an unquoted LLVM identifier does not parse. Every static local in
the tree happened to sit in a global-namespace file, so the whole class of
function was unbuildable and nobody knew.
static_local_namespacedpins it withphp as the oracle.
$out = $cond ? $s : ($out . ',' . $s);— a stringternary's borrowed arm got no retain, so the next iteration freed it, the
allocator returned the same block, and the accumulator silently became the
newest element repeated. Fixed to the half that cannot corrupt (arms retained,
consumers unchanged); teaching the consumers it is owned over-released and was
reverted — see the commit for what a full fix needs.
it, so the previous generation could not build the tree at all. The holder moved
to a global-namespace file.
bin/buildgreen says nothing abouttools/selfhost.sh— the manifest build compilessrc/Runtimeas a library witha flattened namespace, the self-host path takes everything as one module. The
stability gate caught it, five rebuilds in a row.
bin/.gitignoredid not covermanticore_self*, so a 954k-line.llrodeinto a commit.
Docs
docs/superset.mdis new: the whole surfacephpcannot run — concurrency,compile-time attributes, FFI, modules, types, memory — and the rules it lives
under (no oracle ⇒ hand-written expected output, inert under Zend where possible,
zero cost when unused).
README.mdleads with requirements and install instead ofa wall of counters that go stale;
docs/async.mdandROADMAP.mdtrack whatchanged.
Gates
New cases:
async_write_timeout,async_accept_errno,async_accept_idle_park,dns_resolv_conf,static_local_namespaced.Deliberately not here
Item 5 of the review — the 8 MiB per-fiber stack — is a measurement, not a
fix: 10 000 tasks is 80 GB of virtual address space and 10 000 mappings, and
nobody has established where that meets
vm.max_map_countor a container'sulimit -v. Also owed:??has the same ownership hole as the ternary, obj/arrayarms have it with no fix at all, and
tests/aot/run.shstill has no per-casetimeout — a liveness bug hangs the suite instead of failing it.
🤖 Generated with Claude Code