Skip to content

Commit

Permalink
fix: gix clone ssh://... won't deadlock anymore.
Browse files Browse the repository at this point in the history
For `cargo` specifically we now parse stderr to see if permission errors
occour. This links stderr and stdout and we have to pass information from
a supervisor thread that parses stderr to stdout and use the information to
return a custom io error in time.
Now the algorithm is adjusted to never be able to deadlock, as the problem
is inherently racy and somewhat hard to implement it properly especially without
a good test suite built-into `gitoxde` - there are no ssh servers one can easily
spin up cross-platform.
  • Loading branch information
Byron committed Jan 3, 2023
1 parent b81f650 commit 6ba799c
Showing 1 changed file with 18 additions and 2 deletions.
20 changes: 18 additions & 2 deletions git-transport/src/client/blocking_io/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,23 @@ fn supervise_stderr(
Some(err) => Err(err),
None => match res {
Ok(n) if n == wanted => Ok(n),
Ok(n) => self.recv.recv().ok().map(Err).unwrap_or(Ok(n)),
Ok(n) => {
// TODO: fix this
// When parsing refs this seems to happen legitimately
// (even though we read packet lines only and should always know exactly how much to read)
// Maybe this still happens in `read_exact()` as sometimes we just don't get enough bytes
// despite knowing how many.
// To prevent deadlock, we have to set a timeout which slows down legitimate parts of the protocol.
// This code was specifically written to make the `cargo` test-suite pass, and we can reduce
// the timeouts even more once there is a native ssh transport that is used by `cargo`, it will
// be able to handle these properly.
// Alternatively, one could implement something like `read2` to avoid blocking on stderr entirely.
self.recv
.recv_timeout(std::time::Duration::from_millis(5))
.ok()
.map(Err)
.unwrap_or(Ok(n))
}
Err(err) => Err(self.recv.recv().ok().unwrap_or(err)),
},
}
Expand All @@ -153,7 +169,7 @@ fn supervise_stderr(

let (send, recv) = std::sync::mpsc::sync_channel(1);
std::thread::Builder::new()
.name("supervise ssh".into())
.name("supervise ssh stderr".into())
.stack_size(128 * 1024)
.spawn(move || -> std::io::Result<()> {
let mut process_stderr = std::io::stderr();
Expand Down

0 comments on commit 6ba799c

Please sign in to comment.