Skip to content

SpawnWithTimeout derives the process-group isolation decision twice #191

Description

@jcouball

Context

Refactoring follow-up from a review of the v4.0.5 release changes (#163). (Finding F3 from that review.)

Problem

In Commands::SpawnWithTimeout, the decision "did this class isolate the subprocess into its own process group?" is derived twice:

  • #process_group_options computes it directly (timeout can fire, and the caller set neither pgroup nor new_pgroup)
  • #isolated_in_new_process_group? re-derives it from the merged spawn options via #process_group_leader? plus repeated :not_set checks

The two are equivalent in every combination of timeout and caller-supplied pgroup options, but a reader has to prove that. #process_group_leader? also recomputes spawn_options (rebuilding the merged hash) each time it runs on the kill path.

Proposed refactoring

  • Define isolated_in_new_process_group? as !process_group_options.empty?, making #process_group_options the single source of truth for the isolation decision
  • Capture the effective spawn options once at spawn time (memoize or store in #call) so the kill path inspects the options actually used rather than recomputing the merge

#process_group_leader? must keep honoring caller-supplied pgroup: true / pgroup: 0 / new_pgroup: true for the group-kill decision — only the isolation check collapses.

Acceptance criteria

  • Behavior-preserving: no public API or documented behavior changes
  • One place computes whether this class added the process-group option
  • Timeout group-kill, caller-pgroup passthrough, and abandoned-wait cleanup tests pass unchanged

Blocked by

None — can start immediately.

Activity

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions