Fix SFTP prune deadlock when the connection pool has one client#375
Merged
Conversation
SFTPStore.Prune holds a client from the connection pool for the whole walk of the store. Removing an unreferenced chunk went through RemoveChunk, which waits for another client from the pool, so a prune over a store configured with a single connection (-n 1) blocked forever as soon as it found a chunk to delete. Delete chunks over the connection the prune already holds instead. The test doubles the test binary as the ssh command, serving the SFTP protocol on stdin/stdout, so it runs without an SSH server.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SFTPStore.Pruneholds a client from the connection pool for the entire walk of the store, and removing an unreferenced chunk went throughRemoveChunk, which waits for another client from the pool. With a single-connection store (-n 1), the first chunk to delete blocked forever waiting for a connection that could never be returned, hanging theprunecommand indefinitely.The fix deletes chunks over the connection the prune already holds: the removal logic moves to a
removeChunkmethod on the connection (SFTPStoreBase),RemoveChunkkeeps its pool-acquire behavior and delegates, andPrunecalls the method on its held client.Since the repo had no SFTP test coverage (the store needs an SSH server), the new test doubles the test binary as the
sshcommand viaCASYNC_SSH_PATH: when re-executed with a marker environment variable it serves the SFTP protocol on stdin/stdout using the server included in the existinggithub.com/pkg/sftpdependency. The regression test runs a prune against a single-connection store and fails on timeout if the removal deadlocks (verified against the unfixed code). Skipped on Windows, where the URL path can't address the local filesystem served by the test server.