Detach async jobs by forking instead of SIGKILLing the web server

[bjfultn]

What this fixes

Async submits answer the client by killing the web server process that is serving the request.

__printAsyncResponse__ sends the 303, sleeps two seconds, then calls os.kill(os.getppid(), signal.SIGKILL). Under CGI that parent is the web server child handling the request. That does end the response (the body carries no Content-Length, so the dying connection is what terminates the message) and it does orphan the CGI so the query survives. There is no fork anywhere in tap.py; this kill is the detach mechanism.

Bare mod_cgi respawns the killed child silently, so this was invisible for years. Behind a reverse proxy it is not: the proxy is the one holding the connection that dies mid-response, so it marks the backend in error and answers 5xx for the next request or two routed over it.

That matches what we measured against the NEA from outside: a roughly one second window right after PHASE=RUN in which the job's own resources return 503 while /TAP/availability still returns 200 on another worker, hitting about 2 of 9 attempts. The job itself is always fine, which is why polling through the window recovers.

The fix

Fork. 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 it a fast query can write COMPLETED before the parent writes EXECUTING, and the job looks like it went backwards.
• 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 signal an unrelated process.

setsid() matters on its own: mod_cgi registers a cleanup that terminates the CGI child when the request pool is destroyed, so a worker that merely closed stdout would still be killed with the request.

The 303 is also self-delimiting now (Content-Type, Content-Length, Connection: close). 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: the old behavior minus the kill. Correct response, job tied to the request's lifetime.

The two second time.sleep(2.0) is gone with the kill it was covering for, so every async submit returns about two seconds sooner.

Tests

tests/test_async_flow.py covers the three round trips end to end for the first time: submit, PHASE=RUN, poll to COMPLETED, and a non-empty result artifact on disk. It also asserts the 303 is framed, and guards the regression at the source.

That last assertion reads tap.py rather than observing behavior, deliberately: 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. Comments and strings are stripped with tokenize before the check so the code can still explain in prose why it no longer does this.

The async flow was untestable because of this bug, which is also what tests/test_pyneid_compat.py had recorded without identifying: "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; the module stays skipped because pyNEID is not a test dependency.

Three unrelated things had to be fixed before the fixture could host an async job at all, in a separate first commit:

• The composed job URLs were malformed. configparam.py appends HTTP_PORT to HTTP_URL for any port other than 80/443, and the fixture had the port in both, so the status URL came back as host:port:0. No sync test reads httpurl, which is why this never surfaced.
• The fixture handler dropped the CGI's stderr, which makes a 500 from the script indistinguishable from a 500 the script meant to send. It is surfaced on the runner's stderr now, and it is what found the next item.
• tap.py asks BeautifulSoup for the lxml tree builder by name when it reads a status document (TAP/tap.py:904, 2162), so beautifulsoup4 alone is not enough for anything that touches an existing job. lxml is added to requirements-test.txt.

Results, in python:3.12-slim: 58 passed, 2 skipped (53 passed before this branch), ruff check . clean. Both commits are independently green. The async tests were run five times in a row to check the handshake for flakiness.

The first commit also strips one trailing space in TAP/vositables.py that came in with the v3.0.1 merge and is currently failing ruff check . on develop.

Follow-ups this deliberately leaves alone

• setup.py's install_requires lists only ADQL, spatial_index and configobj. It is missing beautifulsoup4, lxml, xmltodict, astropy and sqlparse, all of which tap.py imports at module scope.
• import cgi at TAP/tap.py:5 blocks Python 3.13+, where cgi was removed. It is why the CI matrix stops at 3.12.
• __writeStatusMsg__ opens the status file 'w+' (truncating) and only then takes fcntl.lockf(LOCK_EX|LOCK_NB), so a concurrent reader can see an empty file. This PR does not add any new status writes, partly for that reason.
• __printSyncResponse__ (TAP/tap.py:2695) is defined but never called, and has the same unframed-303 shape.

Note for anyone testing this locally

The suite needs Python 3.12 or older (import cgi) and gcc for the writerecs extension.

---

Submitted by Reid (agent)

🤖 Generated with Claude Code

https://claude.ai/code/session_01Q5cRrMXoPJ6zSiGfKYcqjC

[Copilot]

Copilot review overview

🟡 Changes recommended

Critical async lifecycle issues remain, with additional response-framing and abort-coverage gaps.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

• Avoid replacing workers for terminal job phases · New
• Release child after broken client response · New

What changed in this PR

This pull request replaces SIGKILL-based async detachment with forked workers and adds end-to-end regression coverage.

Changes:

• Adds detached workers, status handshakes, and framed redirects.
• Adds async flow tests and updates pyNEID documentation.
• Fixes fixture configuration, stderr handling, dependencies, and whitespace.

File	Summary
tests/​test_pyneid_compat.py	Updates the skipped-test rationale.
tests/​test_async_flow.py	Adds async submission, execution, polling, and artifact coverage.
tests/​fixtures/​TAP.conf.template	Corrects generated service URLs.
tests/​conftest.py	Improves fixture configuration and stderr handling.
TAP/​vositables.py	Removes trailing whitespace.
TAP/​tap.py	Implements forked async workers and response framing.
requirements-test.txt	Adds the lxml test dependency.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Add the RUN→ABORT PID-handoff test and strengthen validation of the artifact referenced by href.

Review effort: Lite
Findings: None

Resolved since last review (2)

• Release child after broken client response
• Avoid replacing workers for terminal job phases

[tobular]

I ran some tests on https://kvmexoweb.ipac.caltech.edu/ (essentially Centos OPS pre-cutover) and the behavior seems identical to what I'm seeing on RedHat, there are some constraints with the test where you get redirected to RedHat after initiating your query but I wonder how your tests fair with this endpoint?