serve_client: escape the spin, and instrument the worker wedge - #160
Merged
Conversation
serve_client() had no escape condition: it re-calls http_process_client()
with no wait and no timeout, discards the return value, and keys progress
solely on httpc->state -- a field the callee need not touch. httppc() has
exactly such a path, the busy-exit at the top, so a client left in
httpd->busy makes the worker spin on ENQ/DEQ forever and lose itself to
the pool. Count passes that change nothing, report, and force the close
rather than taking the address space down with it.
Three probes for the open investigation:
HTTPD900D the busy-exit fires, naming the client and its state
HTTPD901E the spin guard trips in serve_client()
HTTPD902D http_gets' poll loop runs past its deadline, which separates
"the wait returns but the timeout never fires" from "the wait
never returned at all"
Also records why the storage probe that used to sit in the socket thread
is gone: it bisected with malloc() and abended the thread S878, because
malloc goes to @@getm, which issues GETMAIN RU -- unconditional. On this
platform allocating is not a way to measure free storage.
Refs #159, mvslovers/mvsmf#217, mvslovers/libc370#81
The spin guard rescued the worker but left the cause behind. Reaching it means httppc() kept taking its busy-exit and never got to http_reset_busy(), so the client is still in httpd->busy -- and http_close() only removes from httpd->httpc, never from busy. Without this the freed HTTPC address stays in the array and wedges the next client that happens to be allocated at it. Refs #159
HTTPD900D and HTTPD902D are temporary and should be easy to switch off or delete once #159 is understood. Guard them with a single define so silencing them is a one-character change and removing them is two self-contained blocks plus the define. The spin guard in serve_client() is deliberately left unguarded: it is a fix rather than a diagnostic, and its HTTPD901E message is the whole point of having it. Verified to build clean with the switch in both positions. Refs #159
This was referenced Aug 9, 2026
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.
serve_client()(src/httpd.c) had no escape condition:It re-calls
http_process_client()with no wait and no timeout, discards thereturn value, and keys progress solely on
httpc->state— a field the calleeis not obliged to touch.
httppc()has exactly such a path, the busy-exit atthe top (
if (http_is_busy(httpc)) goto quit;). A client left inhttpd->busytherefore spins the worker forever on ENQ/DEQ, and that worker islost to the pool for the life of the address space.
Measured on a live degraded instance: two workers with their client frozen at
CSTATE_DONEand permanently inhttpd->busy, where every problem-statePSW sample landed in
@@ENQDEQ(i.e.lock()/unlock()) and not one inhttp_done,http_send,recvorfclose. Details in #159.What this changes
A spin guard in
serve_client()— counts passes that leavehttpc->stateunchanged and, past a threshold, reports and forces the close instead of taking
the address space down. It also calls
http_reset_busy()first: reaching theguard means
httppc()kept taking its busy-exit and never got tohttp_reset_busy(), so the client is still inhttpd->busy— andhttp_close()only removes fromhttpd->httpc, never frombusy. Withoutthat call the freed HTTPC address stays in the array and wedges the next client
allocated at it.
Two diagnostics, behind
HTTPD_DEBUG_217(include/httpd.h):HTTPD900DHTTPD902Dhttp_gets' poll loop runs past its deadlineHTTPD902Dexists to separate the two hang shapes: "the wait returns but thedeadline never fires" from "the wait never returned at all". A worker that
wedges with no message from either is stuck inside
cthread_timed_wait(),which would make it a libc370 problem rather than an httpd one.
Removability
Deliberately split, since these have different lifetimes:
HTTPD901Emessage are not behind the switch.They are a fix, not a diagnostic, and are meant to stay.
HTTPD_DEBUG_217to 0 to silence them, ordelete the define together with the two
#if HTTPD_DEBUG_217blocks to dropthem entirely — they are self-contained and touch nothing else.
Verified to build clean under
-Wall -Werrorwith the switch in both positions.Note
There is also a comment in
httpd.crecording why the storage probe thatbriefly lived in the socket thread is gone: it bisected with
malloc()andabended the thread S878, because
mallocreaches@@GETM, which issuesGETMAIN RU— unconditional. On this platform allocating is not a way tomeasure free storage. That is the root cause of #217's S80A cascade and is
filed as mvslovers/libc370#81.
Refs #159, mvslovers/mvsmf#217, mvslovers/libc370#81