Skip to content

Fix a blocking-pool deadlock in nativelink_util::fs::read_dir [1.6-patch-5] - #2647

Open
cormacrelf wants to merge 3 commits into
TraceMachina:mainfrom
cormacrelf:1.6-patch-5
Open

Fix a blocking-pool deadlock in nativelink_util::fs::read_dir [1.6-patch-5]#2647
cormacrelf wants to merge 3 commits into
TraceMachina:mainfrom
cormacrelf:1.6-patch-5

Conversation

@cormacrelf

@cormacrelf cormacrelf commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Description

The previous implementation acquired a blocking-pool permit and then, from inside that blocking-pool thread, called Handle::current().block_on(tokio::fs::read_dir(path)). But tokio::fs::read_dir is itself implemented as a spawn_blocking, so this nested block_on required a second blocking-pool thread to be available to make progress — every call to read_dir needed two pool threads at once instead of one.

Under enough concurrent callers (e.g. upload_directory's unbounded directory-tree fan-out on an action with many inputs), all blocking-pool threads could end up parked waiting on inner tasks that could never get a thread to run on, freezing every fs:: operation in the process. I believe this may have caused deadlocks on actions with a lot of inputs (700+).

Fix: acquire the permit, then simply .await tokio::fs::read_dir directly instead of block_on-ing it — no nested blocking-pool thread is required.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

New regression test read_dir_needs_only_one_blocking_thread in nativelink-util/tests/fs_test.rs: builds a runtime with max_blocking_threads(1) and asserts fs::read_dir completes within a 5s timeout. The pre-fix implementation deadlocks in this configuration since it requires two blocking-pool threads at once.

Checklist

  • Updated documentation if needed
  • Tests added/amended
  • bazel test //... passes locally
  • PR is contained in a single commit, using git amend see some docs

This change is Reviewable

Cormac Relf and others added 3 commits July 31, 2026 10:23
@vercel

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nativelink Ready Ready Preview Jul 31, 2026 5:56am
nativelink-aidm Ready Ready Preview Jul 31, 2026 5:56am

Request Review

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