Skip to content

Detach async jobs by forking instead of SIGKILLing the web server (NEA branch) - #29

Open
bjfultn wants to merge 2 commits into
hotfix/table-validationfrom
fix/async-detach-nea
Open

bjfultn wants to merge 2 commits into
hotfix/table-validationfrom
fix/async-detach-nea

Conversation

@bjfultn

@bjfultn bjfultn commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Port of #28's two code fixes onto the NEA branch, so NEA has a branch to test
against. Branched from hotfix/table-validation at bbc1108 (the #26 merge),
so it already carries the VOSI /capabilities and /availability work.

TAP/tap.py only — no tests, matching this branch's convention. The test
coverage 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, then
os.kill(os.getppid(), SIGKILL). Under CGI that parent is the web server child
serving 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, points
its stdio at /dev/null, waits on a pipe until the parent has published the
status document, then runs the query. uws:runId carries the child's pid, since
ABORT signals whatever runId names.

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 ABORTED and returned results anyway. Terminal phases
now write their status, answer the client, and sys.exit(). Only RUN detaches.

Verification

Ran the full async flow out of process against this branch, with a SQLite
fixture (no server, no proxy):

1. harness process survives the run        alive
2. submit async job                        303, jobid issued
3. POST PHASE=RUN -> poll                  303, phase COMPLETED
4. GET results                             200 (494B)
5. POST PHASE=ABORT                        phase ABORTED, result files: 0
6. VOSI regression check                   /capabilities 200, /availability 200
                                           (one HTTP response each)

Same harness against this branch's base (origin/hotfix/table-validation) dies
at 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

🤖 Generated with Claude Code

Webserver Operations and others added 2 commits October 1, 2026 10:06
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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant