Conversation
An async submit has to finish an HTTP response now and keep executing the query afterwards. Under CGI those pull against each other: the server completes the response when the script closes stdout, and mod_cgi terminates the script once the request is cleaned up. So the process that runs the query can be neither the one holding stdout nor a member of the request's process group. __printAsyncResponse__ resolved that by sending the 303, sleeping two seconds, and then SIGKILLing os.getppid(). Under CGI that parent is the web server child serving the request. Killing it does end the response, since the body carried no Content-Length, and it does orphan the CGI so the job survives. But the client's connection dies mid-message, and behind a reverse proxy the proxy is the one holding that connection: it marks the backend in error and answers 5xx for the next request or two routed over it. Bare mod_cgi respawned the killed child silently, so the damage only became visible once a proxy was put in front. Fork instead. The parent records the child as the job's runId, writes the status document, sends the 303 and exits, which ends the response the way any other CGI does. The child calls setsid() to leave the request's process group, points its standard streams at /dev/null, and runs the query. Three details are load-bearing: - The child waits on a pipe until the parent has published the status document. Without that, a fast query can write COMPLETED before the parent writes EXECUTING, and the job looks like it regressed. - The child redirects stdio before it waits, so it never holds a dup of the server pipe (which would delay the response) and can never write into a response that has already been sent. - uws:runId now carries the child's pid, because ABORT kills whatever runId names (TAP/tap.py:1125-1136). A stale pid there would either do nothing or, worse, signal an unrelated process. The response is also self-delimiting now: an nph- script gets no Content-Length from the server, and a proxy has no other way to know where the message ends. If fork() fails, the request is answered from this process and the query runs here, which is the old behavior minus the kill: correct response, job tied to the request's lifetime. tests/test_async_flow.py covers the full three round trips (submit, PHASE=RUN, poll to COMPLETED plus a non-empty result on disk), asserts the 303 is framed, and guards the regression at the source. That last one is a source assertion on purpose: the behavioral version is "the process running this suite is still alive", which a suite that has been SIGKILLed cannot make. Before this change, running these tests printed "Killed" and nothing else. tests/test_pyneid_compat.py recorded that pyNEID "hung at step 2 with no observable error in the TAP debug log". Step 2 is PHASE=RUN. Its docstring is corrected to say what the hang actually was. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q5cRrMXoPJ6zSiGfKYcqjC
Both from Copilot's review, both real. The async block ends in one shared response path, and every phase reaches it. ABORT and unrecognized phases decide their terminal status, hit that path, and then fall through into the query, which overwrites ABORTED (or ERROR) with COMPLETED. So a client that aborted a PENDING job was told ABORTED and got its results anyway. That predates this branch: the old code wrote the status, sent the 303, killed the server and then ran the query in the orphan. The fork made it easier to see, not worse. The dead per-branch response calls still visible in the RUN and ABORT branches show the original intent, hoisting them into a shared tail is what dropped the exit. Terminal phases now write their status, answer the client, and sys.exit(), which is what __writeAsyncError__ and __printError__ already do everywhere else in this file. Only RUN detaches. Second: the parent released the child after sending the response, so a client or proxy that hung up mid-write took the exception path, skipped the release, and the child saw EOF and exited, leaving a published job stuck in EXECUTING forever. The release moves into a finally and is gated on the status write having succeeded: a hung-up client does not make the job go away, but if there is no job document there is nothing for the child to update. Tests cover both terminal phases, including that nothing runs afterwards (phase holds and no result file appears). Both fail against the previous commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q5cRrMXoPJ6zSiGfKYcqjC
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Port of #28's two code fixes onto the NEA branch, so NEA has a branch to test
against. Branched from
hotfix/table-validationatbbc1108(the #26 merge),so it already carries the VOSI
/capabilitiesand/availabilitywork.TAP/tap.pyonly — no tests, matching this branch's convention. The testcoverage for these fixes lives on #28 against
develop.What changed
1. Async submit forks instead of killing the web server.
__printAsyncResponse__used to send the 303, sleep two seconds, thenos.kill(os.getppid(), SIGKILL). Under CGI that parent is the web server childserving the request. Killing it truncates the response, and behind a reverse
proxy it leaves the proxy holding a dead upstream connection — which the proxy
reports as 5xx on the next request or two routed over it.
The submit path now forks: the parent publishes the job, sends a framed 303 and
exits normally; the child
setsid()s out of the request's process group, pointsits stdio at
/dev/null, waits on a pipe until the parent has published thestatus document, then runs the query.
uws:runIdcarries the child's pid, sinceABORT signals whatever
runIdnames.2. ABORT and unrecognized phases no longer run the job.
Every phase fell through one shared response path and then into the query, so
an aborted job reported
ABORTEDand returned results anyway. Terminal phasesnow write their status, answer the client, and
sys.exit(). OnlyRUNdetaches.Verification
Ran the full async flow out of process against this branch, with a SQLite
fixture (no server, no proxy):
Same harness against this branch's base (
origin/hotfix/table-validation) diesat step 3 with exit 137 — SIGKILL — which is the bug: the CGI killing the
process that invoked it. That is the red half of the check; the fix is the green.
Step 5 guards the second fix: before it, an aborted job still produced a result
file.
Notes
303 See Otheremitters intap.pyare untouchedhere, exactly as in Detach async jobs by forking instead of SIGKILLing the web server #28. Only
__printAsyncResponse__gainedContent-Lengthand
Connection: close.don't exist on
hotfix/table-validation.🤖 Generated with Claude Code