Skip to content

Filestore update deadlock - #2007

Merged
chrisstaite-menlo merged 1 commit into
TraceMachina:mainfrom
chrisstaite:update_deadlock
Oct 28, 2025
Merged

Filestore update deadlock#2007
chrisstaite-menlo merged 1 commit into
TraceMachina:mainfrom
chrisstaite:update_deadlock

Conversation

@chrisstaite

@chrisstaite chrisstaite commented Oct 23, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

If there is a lot of files being created at the same time then there might be the case that the populator is attempting to write to a DropCloseWriterHalf but the DropCloseReaderHalf is blocked waiting for a file system semaphore. These are held by the FileSlot in FileSystemStore while it populates the temp file. This can lead to a deadlock where the readers are holding semaphores on the paths that don't have FileSlots and the ones that do have FileSlots don't have download semaphores.

Ensure that the reader has the first chunk of data before taking the FileSlot. If the reader starts then we assume that we have all the permits we need to finish the file, therefore it should be safe to take the FileSlot semaphore.

Type of change

Please delete options that aren't relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Added a test that deadlocks without the change. It may not be the cause of #2001 but it certainly can't hurt.

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

@chrisstaite
Filestore update deadlock
260cc8d 
If there is a lot of files being created at the same time then there might be the case that the populator is attempting to write to a DropCloseWriterHalf but the DropCloseReaderHalf is blocked waiting for a file system semaphore.  These are held by the FileSlot in FileSystemStore while it populates the temp file.  This can lead to a deadlock where the readers are holding semaphores on the paths that don't have FileSlots and the ones that do have FileSlots don't have download semaphores.

Ensure that the reader has the first chunk of data before taking the FileSlot.  If the reader starts then we assume that we have all the permits we need to finish the file, therefore it should be safe to take the FileSlot semaphore.
@chrisstaite-menlo
chrisstaite-menlo enabled auto-merge (squash) October 24, 2025 13:56
@chrisstaite-menlo
chrisstaite-menlo merged commit d55c59d into TraceMachina:main Oct 28, 2025
30 checks passed
@palfrey palfrey mentioned this pull request Nov 5, 2025
Open
rejuvenile pushed a commit to rejuvenile/nativelink that referenced this pull request Jul 13, 2026
@chrisstaite
Filestore update deadlock (TraceMachina#2007)
2539806 
If there is a lot of files being created at the same time then there might be the case that the populator is attempting to write to a DropCloseWriterHalf but the DropCloseReaderHalf is blocked waiting for a file system semaphore.  These are held by the FileSlot in FileSystemStore while it populates the temp file.  This can lead to a deadlock where the readers are holding semaphores on the paths that don't have FileSlots and the ones that do have FileSlots don't have download semaphores.

Ensure that the reader has the first chunk of data before taking the FileSlot.  If the reader starts then we assume that we have all the permits we need to finish the file, therefore it should be safe to take the FileSlot semaphore.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@amankrx amankrx amankrx approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@chrisstaite @amankrx @chrisstaite-menlo