Skip to content

[Fix] Stop the web app's whole process group on cleanup - #89

Open
aaronsmulktis wants to merge 4 commits into
mainfrom
aaronsmulktis/stop-app-process-group
Open

aaronsmulktis wants to merge 4 commits into
mainfrom
aaronsmulktis/stop-app-process-group

Conversation

@aaronsmulktis

Copy link
Copy Markdown
Contributor

Why this PR?

Every launch_agent.py run left its OpenApps web server running after it exited. After a day of local runs I had nine orphaned servers holding ports 5001–5901, each parented to launchd.

The web app is started as bash -c "source … && cd … && uv run launch.py …" with start_new_session=True. cleanup() called apps_process.terminate(), which signals only that bash wrapper. bash exits, and uv and the server under it are reparented to init and keep serving.

The leaked servers pile up. In a conduct.sh pool they collect across runs for the life of the allocation, and every port they hold pushes later runs further along their search. That port pressure is how #88's collisions showed up.

What changes

  • New stop_process_group() in launcher.py sends SIGTERM to the whole process group, then SIGKILL after a timeout. It waits on the group, not on bash, because bash can exit long before the server does. It reaps the wrapper as it goes, since an unreaped zombie still counts as a group member. This matches how tests/save_screenshots.py already stops its server.
  • AgentLauncher.launch() now stops the app in a finally. Two exits used to skip cleanup entirely: a server that never came up (wait_until_apps_start raising), and Ctrl-C (KeyboardInterrupt slipped past except Exception).
  • The kill_ports fallback after a timeout is gone. Killing the group covers it, and killing by port number could hit another run's server on a reused port.

Test plan

  • New tests/test_launcher_cleanup.py:
    • a server under a bash -c "… && child" wrapper is gone after cleanup;
    • a server that ignores SIGTERM is SIGKILLed;
    • an already-exited process is a no-op.
  • The old terminate() against the same setup leaves the child running, so the first test catches the regression.
  • test_launcher_recording.py and test_parallel_tasks.py: 27 passed.
  • uv run launch_agent.py agent=dummy task_name=add_meeting_with_dennis ends with "OpenApps successfully stopped". Afterwards nothing OpenApps is listening and no launch.py process remains.
  • Ruff: no new findings; one fewer, since signal is now used.

The web app runs as bash -c "... && uv run launch.py" in its own session, and cleanup() called terminate() on the bash wrapper only. bash exited; uv and the server were reparented to init and kept serving, so every launch_agent run left a server holding a port. Signal the process group instead (SIGTERM, then SIGKILL), waiting on the group rather than on bash. Also stop the app when the server never comes up or on Ctrl-C, which escaped except Exception.
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 14, 2026
@aaronsmulktis aaronsmulktis self-assigned this Oct 19, 2026

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

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant