Skip to content

Fix nested Tokio runtime panic in subprocess bridge - #828

Merged
zackees merged 1 commit into
mainfrom
fix/nested-runtime-blocking-subprocess
Jun 29, 2026
Merged

Fix nested Tokio runtime panic in subprocess bridge#828
zackees merged 1 commit into
mainfrom
fix/nested-runtime-blocking-subprocess

Conversation

@zackees

@zackeeszackees commented Jun 29, 2026

Copy link
Copy Markdown
Member

Summary

  • allow blocking subprocess helpers to run from an existing multithreaded Tokio runtime via block_in_place
  • keep plain sync callers on a small current-thread runtime
  • add a regression test for the Tokio worker path

Closes#827

Tests

  • soldr cargo fmt --all -- --check
  • soldr cargo test -p fbuild-core run_command_blocking_works_from_tokio_worker -- --nocapture
  • soldr cargo test -p fbuild-core

Summary by CodeRabbit

  • Bug Fixes

    • Improved command execution when called from inside an active Tokio runtime, avoiding nested runtime issues.
    • run_command_blocking now works correctly from multithreaded Tokio worker tasks and returns expected output.
  • Tests

    • Added coverage for invoking blocking command execution within a Tokio task.

@coderabbitai

coderabbitaiBot commented Jun 29, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f415e433-2cf4-412c-90be-8760d0d65545

📥 Commits

Reviewing files that changed from the base of the PR and between 9445e7f and f50ec01.

📒 Files selected for processing (1)
  • crates/fbuild-core/src/subprocess.rs

📝 Walkthrough

Walkthrough

block_on in subprocess.rs is updated to detect an existing Tokio runtime via Handle::try_current() and use block_in_place when one is present, instead of always constructing a new current-thread runtime. Documentation is updated to describe both paths, and a new multi-thread Tokio test verifies run_command_blocking succeeds from a worker task.

Subprocess blocking shim fix

Layer / File(s)Summary
block_on runtime detection and regression test
crates/fbuild-core/src/subprocess.rs
block_on checks Handle::try_current() and runs via block_in_place when inside a multithreaded Tokio runtime; falls back to a new current-thread runtime for plain sync callers. Docs updated to describe both paths. New #[tokio::test(flavor = "multi_thread")] test asserts run_command_blocking completes without panicking from a worker task.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A rabbit once blocked in a loop,
Two runtimes caused quite a flop!
Now try_current saves the day,
block_in_place clears the way,
No more panics — hip hop hooray! 🎉

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/nested-runtime-blocking-subprocess

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@zackees
zackees merged commit ba50fc5 into mainJun 29, 2026
84 of 91 checks passed
@zackees
zackees deleted the fix/nested-runtime-blocking-subprocess branch June 29, 2026 17:25
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

Daemon can panic by starting nested Tokio runtime from blocking subprocess helper

1 participant

@zackees