Skip to content

(via norrietaylor): Fix trap handlers to prevent orphaning background processes in bulk-uplo - #891

Merged
macroscopeapp[bot] merged 1 commit into
test/bulk-upload-canary-869from
macroscope/test/bulk-upload-canary-869
Jul 22, 2026
Merged

(via norrietaylor): Fix trap handlers to prevent orphaning background processes in bulk-uplo#891
macroscopeapp[bot] merged 1 commit into
test/bulk-upload-canary-869from
macroscope/test/bulk-upload-canary-869

Conversation

@macroscopeapp

@macroscopeapp macroscopeapp Bot commented Jul 22, 2026

Copy link
Copy Markdown

Summary

This PR fixes a process management issue in bulk-upload-e2e.sh where background processes could be orphaned when the script exits unexpectedly (e.g., via signal or timeout).

Changes

  • Added CUR_PID global variable to track the currently running background process
  • Added cleanup_child() function that kills the tracked background process if it exists
  • Updated all three trap handlers to call cleanup_child before teardown
  • Modified run_deadline() to:
    • Set CUR_PID=$pid after spawning the background process
    • Clear CUR_PID before both return paths (success and timeout)

Why

Previously, if the script was interrupted or hit a timeout, background processes spawned by run_deadline() could continue running as orphans. This fix ensures proper cleanup by tracking and terminating any active background process when the script exits.

Note

Macroscope: Fix It For Me

  • This PR originated from this comment in test(scripts): canary the bulk host-to-guest upload path #887.
  • Since auto-merge is on, Macroscope will merge this PR after waiting for checks to pass.
  • If you'd rather not wait, you can always merge this yourself but no further action from you is currently needed.
  • You can also @mention Macroscope in this PR to request further changes.

Activity

Currently: Auto-merged a827cb4

Previously

Note

Fix trap handlers in bulk-upload e2e script to prevent orphaned background processes

  • Adds a cleanup_child helper in bulk-upload-e2e.sh that sends a termination signal to a tracked child process (stored in CUR_PID) before teardown runs.
  • Updates EXIT, INT, and TERM traps to call cleanup_child before teardown, ensuring background processes are killed on all exit paths.
  • The run_deadline function now sets CUR_PID after backgrounding a command and clears it after the process exits or is force-killed at deadline.

Macroscope summarized 23c9dd9.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: ec0b0b2b-e912-4794-bd8f-4b7af6f71cf8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@macroscopeapp
macroscopeapp Bot merged commit a827cb4 into test/bulk-upload-canary-869 Jul 22, 2026
2 checks passed
@macroscopeapp
macroscopeapp Bot deleted the macroscope/test/bulk-upload-canary-869 branch July 22, 2026 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant