Skip to content

Commit bd7fe89

Browse files
committed
Fix a deadlocking test with master libgit2
This commit fixes a test in Cargo to work around a seeming regression in behavior in libgit2 around HTTP authentication. The expected flow for HTTP authentication with git is that git sends an HTTP request and receives an "unauthorized" response. It then sends another request with authorization information and that's what we're testing is received in the our test. Previously libgit2 would issue a new HTTP connection if the previous one was closed, but it looks like changes in libgit2 now require that the same HTTP connection is used for the initial request and the subsequent request with authorization information. This broke our test since it's not using an HTTP compliant server at all and is just some handwritten TCP reads/writes. The fix here is to basically stay with handwritten TCP reads/writes but tweak how it happens so it's all on the same HTTP/TCP connection to match what libgit2 is expecting. Some extra assertions have also been added to try to prevent deadlocks from happening in the future and instead make the test fail fast if this situation comes up again.
1 parent caba711 commit bd7fe89

File tree

2 files changed

+20
-14
lines changed

2 files changed

+20
-14
lines changed

Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ features = [
100100
]
101101

102102
[dev-dependencies]
103-
bufstream = "0.1"
104103
cargo-test-macro = { path = "crates/cargo-test-macro", version = "0.1.0" }
105104

106105
[[bin]]

tests/testsuite/build_auth.rs

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1-
use std;
21
use std::collections::HashSet;
32
use std::io::prelude::*;
3+
use std::io::BufReader;
44
use std::net::TcpListener;
5+
use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
6+
use std::sync::Arc;
57
use std::thread;
68

79
use crate::support::paths;
810
use crate::support::{basic_manifest, project};
9-
use bufstream::BufStream;
1011
use git2;
1112

1213
// Tests that HTTP auth is offered from `credential.helper`.
@@ -25,15 +26,20 @@ fn http_auth_offered() {
2526
.collect()
2627
}
2728

29+
let connections = Arc::new(AtomicUsize::new(0));
30+
let connections2 = connections.clone();
2831
let t = thread::spawn(move || {
29-
let mut conn = BufStream::new(server.accept().unwrap().0);
32+
let mut conn = BufReader::new(server.accept().unwrap().0);
3033
let req = headers(&mut conn);
31-
conn.write_all(
32-
b"HTTP/1.1 401 Unauthorized\r\n\
34+
connections2.fetch_add(1, SeqCst);
35+
conn.get_mut()
36+
.write_all(
37+
b"HTTP/1.1 401 Unauthorized\r\n\
3338
WWW-Authenticate: Basic realm=\"wheee\"\r\n\
39+
Content-Length: 0\r\n\
3440
\r\n",
35-
)
36-
.unwrap();
41+
)
42+
.unwrap();
3743
assert_eq!(
3844
req,
3945
vec![
@@ -44,16 +50,16 @@ fn http_auth_offered() {
4450
.map(|s| s.to_string())
4551
.collect()
4652
);
47-
drop(conn);
4853

49-
let mut conn = BufStream::new(server.accept().unwrap().0);
5054
let req = headers(&mut conn);
51-
conn.write_all(
52-
b"HTTP/1.1 401 Unauthorized\r\n\
55+
connections2.fetch_add(1, SeqCst);
56+
conn.get_mut()
57+
.write_all(
58+
b"HTTP/1.1 401 Unauthorized\r\n\
5359
WWW-Authenticate: Basic realm=\"wheee\"\r\n\
5460
\r\n",
55-
)
56-
.unwrap();
61+
)
62+
.unwrap();
5763
assert_eq!(
5864
req,
5965
vec![
@@ -144,6 +150,7 @@ Caused by:
144150
))
145151
.run();
146152

153+
assert_eq!(connections.load(SeqCst), 2);
147154
t.join().ok().unwrap();
148155
}
149156

0 commit comments

Comments
 (0)