From bd7fe8914bce7b5641b45b55a643a71a8c20df82 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Thu, 25 Jul 2019 09:07:25 -0700
Subject: [PATCH] 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.
---
 Cargo.toml                    |  1 -
 tests/testsuite/build_auth.rs | 33 ++++++++++++++++++++-------------
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 2592905ed6a..8c1367b100c 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -100,7 +100,6 @@ features = [
 ]
 
 [dev-dependencies]
-bufstream = "0.1"
 cargo-test-macro = { path = "crates/cargo-test-macro", version = "0.1.0" }
 
 [[bin]]
diff --git a/tests/testsuite/build_auth.rs b/tests/testsuite/build_auth.rs
index c2b59585be1..2509f90a32f 100644
--- a/tests/testsuite/build_auth.rs
+++ b/tests/testsuite/build_auth.rs
@@ -1,12 +1,13 @@
-use std;
 use std::collections::HashSet;
 use std::io::prelude::*;
+use std::io::BufReader;
 use std::net::TcpListener;
+use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
+use std::sync::Arc;
 use std::thread;
 
 use crate::support::paths;
 use crate::support::{basic_manifest, project};
-use bufstream::BufStream;
 use git2;
 
 // Tests that HTTP auth is offered from `credential.helper`.
@@ -25,15 +26,20 @@ fn http_auth_offered() {
             .collect()
     }
 
+    let connections = Arc::new(AtomicUsize::new(0));
+    let connections2 = connections.clone();
     let t = thread::spawn(move || {
-        let mut conn = BufStream::new(server.accept().unwrap().0);
+        let mut conn = BufReader::new(server.accept().unwrap().0);
         let req = headers(&mut conn);
-        conn.write_all(
-            b"HTTP/1.1 401 Unauthorized\r\n\
+        connections2.fetch_add(1, SeqCst);
+        conn.get_mut()
+            .write_all(
+                b"HTTP/1.1 401 Unauthorized\r\n\
               WWW-Authenticate: Basic realm=\"wheee\"\r\n\
+              Content-Length: 0\r\n\
               \r\n",
-        )
-        .unwrap();
+            )
+            .unwrap();
         assert_eq!(
             req,
             vec![
@@ -44,16 +50,16 @@ fn http_auth_offered() {
             .map(|s| s.to_string())
             .collect()
         );
-        drop(conn);
 
-        let mut conn = BufStream::new(server.accept().unwrap().0);
         let req = headers(&mut conn);
-        conn.write_all(
-            b"HTTP/1.1 401 Unauthorized\r\n\
+        connections2.fetch_add(1, SeqCst);
+        conn.get_mut()
+            .write_all(
+                b"HTTP/1.1 401 Unauthorized\r\n\
               WWW-Authenticate: Basic realm=\"wheee\"\r\n\
               \r\n",
-        )
-        .unwrap();
+            )
+            .unwrap();
         assert_eq!(
             req,
             vec![
@@ -144,6 +150,7 @@ Caused by:
         ))
         .run();
 
+    assert_eq!(connections.load(SeqCst), 2);
     t.join().ok().unwrap();
 }