fix(async): close #813 audit gaps (#815/#817/#818) + confirm #816/#819/#820/#821#842
Conversation
Three parallel fix-ups completing the whole-app tokio runtime migration audit: #815 fbuild-daemon — emulator handler fs/wraps: - avr8js_deploy.rs: 5 std::fs::* calls → tokio::fs::*.await - avr8js_web.rs: 2 std::fs::read_to_string → tokio::fs::*.await; load_session_manifest flipped sync → async; in-file callers updated - qemu_deploy.rs: std::fs::create_dir_all → tokio::fs::*.await - avr8js_npm.rs: ensure_avr8js_npm_in inner create_dir_all converted; prepare_avr8js_cache_for_install kept sync (test-callable) and its sole async caller wraps it in spawn_blocking - export_artifacts_bundle (3 call sites in build.rs + deploy.rs) wrapped in spawn_blocking so its sync std::fs::* I/O doesn't run on the axum worker #817 fbuild-python — 7 sync helpers folded into block_on(async_version): - serial_monitor.rs: reset_device now reuses post_reset_request_async - daemon.rs: verify_broker_daemon_cache_identity, ensure_running_via_broker, ensure_running, stop, status all share async helpers - outcome.rs: send_op now folds into send_op_async - ~30-line duplicated sync HTTP blocks collapsed to 1-2 lines each; no more reqwest::blocking::* in fbuild-python/src/ #818 cross-cutting — BuildLog channel flipped to tokio: - BuildLog.sender / BuildParams.log_sender: Option<std::sync::mpsc::Sender<String>> → Option<tokio::sync::mpsc::UnboundedSender<String>> - daemon build handler bridge task (sync-mpsc → tokio-mpsc forwarder on spawn_blocking) deleted; replaced with a direct tokio::spawn forwarder that awaits log_rx.recv() - Test fixtures updated to use tokio::sync::mpsc::unbounded_channel Closes #815, #816, #817, #818, #819, #820, #821 (all sub-issues of #813 confirmed addressed; #816/#819/#820/#821 had no remaining work by verification, #815/#817/#818 fixed here). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Releases the post-#813 / #802 / #826 / #844 work landed across: - PR #843 — workspace-wide timeout sweep (#802 family) - PR #837, #849, #850, #851 — internal-bridges + dylint sweep (#826 + #844) - PR #842 — async migration follow-ups (#813) 15 dylints now ship in dylints/ (4 from earlier, 10 from #826/#844, plus the renamed ban_unwrap_in_production). 7 internal bridge APIs: fbuild_core::{http, fs, time, channel, path::canonicalize_existing}, fbuild_paths::temp_subdir, fbuild_cli::output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Final follow-up to #813's whole-app tokio runtime migration. PR #823 already landed the bulk of the conversion; this PR closes the last three gap-laden audit sub-issues and confirms the four that verified clean.
Verification + fixes by sub-issue
Verified clean (no work needed, closing on this PR):
subprocess.rsCRITICAL paths all async with_blockingshim escape hatch;response_file.rsasync. Secondary findings (config loaders, library-select cache, install_status callback) are extension items beyond the core async migration — deferred.Deployer::deploy+post_deploy_recoveryasync via#[async_trait]; all 4 per-platform deployers async; esptool/avrdude/picotool/pyocd all route throughsubprocess::run_command(...).await.compile_sourceasync with direct.awaitonZccacheService::compile;parallel.rsusesJoinSet/Semaphore;compile_many::run_stage2async;compile_blockingdeprecated.OnceLock<reqwest::Client>HTTP layer; allPackage::ensure_installedasync via#[async_trait]across 31 impls;library_managerusesJoinSet; noreqwest::blocking::*references remain.Gaps fixed in this PR:
audit: sync code that could be async in fbuild-daemon (sub-issue of #813) #815 fbuild-daemon — daemon emulator handlers had 8+ sync
std::fs::*calls running on the axum worker;export_artifacts_bundle(with sync I/O) was called outsidespawn_blockingfrom build + deploy handlers. Fixed: converted 4 emulator handler files totokio::fs::*(one helper flipped sync → async); wrapped 3export_artifacts_bundlecall sites inspawn_blocking. The broker IPC backend (broker/backend.rs) keeps itsstd::thread::spawnmodel intentionally — it's a sync IPC protocol, not on the tokio runtime by design.audit: sync code that could be async in fbuild-cli + fbuild-python (sub-issue of #813) #817 fbuild-python — 7 sync helpers still had ~30-line
reqwest::blocking::Clientblocks duplicated alongsideAsync*counterparts. Folded each into 1-2 linert.block_on(async_version(...))wrappers; extracted 4 shared async helpers; deleted unused intermediate sync wrappers. Zeroreqwest::blocking::*references remain infbuild-python/src/.audit: cross-cutting async/sync architecture (sub-issue of #813) #818 cross-cutting — last structural piece was
BuildLog.sender: Option<std::sync::mpsc::Sender<String>>requiring aspawn_blockingbridge in the daemon's build handler. Flipped totokio::sync::mpsc::UnboundedSender<String>and deleted the 10-line bridge task; the WebSocket forwarder now awaitslog_rx.recv()directly.Test plan
Closes
🤖 Generated with Claude Code