Skip to content

Bug: Verify engine process ownership before terminating a stored PID #103

Closed

Description

@BerryUIKI

Summary

supervisor.py:65 and webui_supervisor.py:67 accept a Windows executable name containing python as ownership evidence. Both accepted the probe's unrelated Python process. Stop subsequently uses taskkill /F /T.

Environment and evidence

  • Review finding: F03 (2026-10-05 product/technical review).
  • Baseline: Windows, dev at c1c5dbe. The remote dev matched this commit when filing.
  • Evidence: Reproduced.
  • Priority recommendation: P1 - proposed first-release blocker. This is review triage, not a production-incident severity declaration.
  • No real credentials, paid inference, engine installation, or unrelated process termination were used for review probes. Mocked observations establish the stated code behavior, not live-provider/GPU acceptance.

Reproduction or validation

On Windows, run an isolated Python process and pass os.getpid() to ComfySupervisor._verify_process_identity and WebUISupervisor._verify_process_identity. Both returned True for the unrelated probe process. Do not call stop or taskkill during this validation; no real process was terminated by the probe.

User impact

A stale PID file can cause Berry to stop an unrelated Python process and its children, contrary to the external-process ownership requirement.

Proposed approach

Verify executable path, command line/runtime identity, creation time, and ownership metadata before any termination; treat ambiguity as unowned. Test recycled PIDs without killing a real process.

Acceptance criteria

  • Verify executable/runtime path, process creation time, command line, and ownership before termination.
  • A recycled or ambiguous PID is treated as unowned.
  • Stopping an owned engine does not affect external engines or unrelated Python processes.
  • Tests simulate stale PID files without terminating real processes.

Verification scope

Real GPU inference, paid-provider compatibility, and a clean-machine packaged desktop journey remain unverified. Any follow-up implementation should target dev under the repository's contribution/branching rules.

Activity

  1. added
    bugSomething isn't working
    criticalCritical priority issues that block core functionality or cause vulnerabilities
    securitySecurity vulnerabilities and risks
    backendBackend Python / FastAPI / Engine issues
    on Oct 4, 2026
  2. BerryUIKI commented on Oct 4, 2026

    @BerryUIKI
    OwnerAuthor

    Resolved by PR #147: Implemented strict executable path and runtime ownership verification in ComfySupervisor, WebUISupervisor, and LlamaServerSupervisor, ensuring foreign or recycled PIDs are safely identified as unowned and never terminated via taskkill/kill, and added regression tests.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    backendBackend Python / FastAPI / Engine issuesbugSomething isn't workingcriticalCritical priority issues that block core functionality or cause vulnerabilitiessecuritySecurity vulnerabilities and risks

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions