Skip to content

fix(math_with_code): release the Python worker in close_resources_session - #5481

Open
yaoyu-33 wants to merge 21 commits into
yuya/shared-resources-lifecyclefrom
yuya/gymint-175-math-with-code-close-sessions
Open

yaoyu-33 wants to merge 21 commits into
yuya/shared-resources-lifecyclefrom
yuya/gymint-175-math-with-code-close-sessions

Conversation

@yaoyu-33

@yaoyu-33 yaoyu-33 commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Release the Python worker through the shared resources lifecycle so close works independently of verification. Switch worker start from the default multiprocessing context (fork on Linux) to a lazily started forkserver worker, keep request-based scoring and serialize allocation/cleanup.

Depends on #5709 (which depends on #5708). The child diff contains only this environment and its related documentation/configuration; shared implementation is reviewed once in the dependency. No separate issue is needed for this focused migration of an existing resources server.

  • Track the shielded worker-start task until its result is published, including cancellation of the request awaiting it. Close waits for pending allocation and then releases the actual worker; an abandoned to_thread result can no longer leak a process.
  • Add forkserver socket isolation and an async close that escalates SIGTERM then SIGKILL off the executor. A surviving process remains owned and makes cleanup fail for retry. Verification now uses the shared public context; bounded receipts retain identity, while scoring recomputes from the request.

Request-scored: retains an empty marker (retain(None)) because the request already carries the answer and grading data.

Validation on 7cc1eb7c8fece27c1302991bd4e7e748976dc3c1, Python 3.13.14, with this checkout first on PYTHONPATH and NEMO_GYM_EXTRA_ROOTS pointing to it:

Real-model smoke on preceding migration revision 27a4d4998ba824d9dc06ac41f11a914cffff9d67 (shared core 4e92d4891) used nvidia/nemotron-3.5-lightning-30b-a3b, Python 3.13.14 and actual forkserver workers. Normal and skip-verification cases each executed Python and returned 385. Normal verification scored 1.0; the skip case made zero verify calls and returned fixed reward 0.0. Verify/close returned 200 as applicable, worker death was observed, and active/starting/identity maps were empty. The skip case released a real live worker at close. Loaded source hashes matched that revision. These are synthetic lifecycle tasks, not a math benchmark baseline. The smoke made one verify per episode, so the retry path (retain(None)) is covered by unit tests only.

These targeted checks do not replace the repository CI matrix or approval of the current head.

Compatibility

The first worker per server process waits for the forkserver to import the module (seconds; later workers start in milliseconds). The forkserver is one extra resident process per server process (about 230 MB measured on macOS). Closing a worker that ignores SIGTERM can take about 11 s while holding the session lock. A worker that cannot start still answers the tool call with success: false (as on main), is now logged, and the episode is scored from the answer alone.

…sion

math_with_code keeps a per-rollout Python worker process keyed by the
session cookie, and stopped it only in /verify or /end_session. Under
single_agent_turn_legacy, an episode that never reached /verify kept its
worker until the server exited, because the inherited
close_resources_session kept no state. Stopping a worker also only joined
for one second, so a worker busy in user code outlived the stop.

- seed_session binds a typed seed's resources_session_id to the cookie and
  records its episode and task; the legacy /run seed body keeps no state.
- close_resources_session stops the worker by resources_session_id, fences
  late seeds with ResourcesSessionFence, is idempotent, and raises when the
  worker survives, so the caller can retry.
- _SessionHandle.close escalates to SIGTERM and SIGKILL and raises if the
  process is still alive; a worker that survives stays registered. Verify
  logs that failure and keeps its score. Stops run off the event loop.
- execute_python for a closed session gets ResourcesSessionLostError
  (404); a call that a close overtook stops the worker it started.
- A /verify for a closed session returns lost_session_verification.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Nov 3, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@yaoyu-33 yaoyu-33 added area:environment Individual environments, benchmarks, verifiers, and environment-specific resources servers bug Something isn't working labels Nov 3, 2026
@yaoyu-33

yaoyu-33 commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test d87a7f4

yaoyu-33 and others added 3 commits November 3, 2026 11:05
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
… the registry on the loop

Linux forks the worker, so it inherits the server's Python SIGTERM handler (uvicorn's under a served app). terminate() then only ran that handler, and every close of a busy worker waited out both joins before SIGKILL, holding a thread and the session's fence for about 6 s. The worker now resets SIGTERM and SIGINT to their defaults.

_cleanup_session ran entirely in a thread, so two cleanups of one handle could both pass the identity check and the second del raised KeyError, which verify's except RuntimeError did not catch. Only handle.close now runs in a thread; the check and delete happen on the event loop.

Adds a test that forks a worker under a no-op SIGTERM handler (fails on the previous code), and moves the conformance import to nemo_gym.server_runtime.testing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
@yaoyu-33

yaoyu-33 commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test af49dc9

@nemo-automation-bot
nemo-automation-bot Bot requested a review from a team November 3, 2026 21:43
@github-actions github-actions Bot added the sla:triage-overdue Review assignment is over the one-business-day SLA label Nov 4, 2026
yaoyu-33 and others added 6 commits November 4, 2026 15:19
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
…ycle

- The repo contract test counts a server that implements the state hooks as
  closing through the inherited close_resources_session. newton_bench,
  workplace_assistant, tavily_search and indirect_prompt_injection override
  seed_session but close through the hooks, and failed it.
- Export reject_partial_typed_seed from nemo_gym.server_runtime, so servers
  that override seed_session reuse the shared guard instead of a copy.
- The MCP seed wrapper still recognises a request param annotated as
  Annotated[Request, ...]; keeping Annotated metadata had made it inject a
  second Request and fail with KeyError.
- Name the 409 for a typed seed over a cookie session "cookie-seeded", not
  "legacy", and document that a served seed_session override declares
  request before body.
- Shared tests now cover the MCP-wrapped seed route, close after verify
  (checked against the verified episode), and a cookie seed of a verified
  typed session.
- The add-benchmark skill describes the three state hooks as the default
  for a stateful server.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
The inherited retire_session_state recorded a typed session as closed. The
checkpoint participant also calls that hook to undo a restore
(delete_restored) and to drop an unclaimed session, and both promise to
refuse nothing later; with the fence, a restore that was rolled back and
installed again in the same process left its session fenced, so its verify
was masked as lost and its continuation's seed got 409. The participant
already refuses a retired attempt's requests until it forgets the rollout,
so retirement now releases the state and drops the identity without
recording a close. A close still fences.

Also write down that an exported server using the state hooks carries each
session's identity through its checkpoint itself.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
…antics

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
@yaoyu-33
yaoyu-33 changed the base branch from main to yuya/shared-resources-lifecycle November 5, 2026 23:29
yaoyu-33 and others added 4 commits November 5, 2026 16:27
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
…start failures, test /end_session

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
@yaoyu-33

yaoyu-33 commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 7cc1eb7

The docs now describe only the typed Environment Server path. The cookie
seed still works unchanged; the PR description records that compatibility.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
@yaoyu-33

yaoyu-33 commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test d11dc70

This branch was previously deployed

1 inactive deployment
public — d11dc706 Deployed Nov 6, 2026 by copy-pr-bot[bot] via release / finalize / notify #10694
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:environment Individual environments, benchmarks, verifiers, and environment-specific resources servers bug Something isn't working needs-review PR is ready for code review and waiting on a reviewer sla:triage-overdue Review assignment is over the one-business-day SLA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant