Reduce flakiness and document reasons for flakiness

Android has been receiving reports of some tests being flaky
on what are probably lower-spec devices.

This introduces delays into tests where sockets are being
poisoned after the entire response body has been written to
them *and* where there are follow-up requests.

This change also improves the documentation for the problematic
SocketPolicy values.

mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockWebServer.java

@@ -477,6 +477,7 @@ public final class MockWebServer {
           writeHttpResponse(socket, sink, response);
         }
 
+        // See warnings associated with these socket policies in SocketPolicy.
         if (response.getSocketPolicy() == SocketPolicy.DISCONNECT_AT_END) {
           source.close();
           sink.close();

mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/SocketPolicy.java

@@ -16,7 +16,19 @@
 
 package com.squareup.okhttp.mockwebserver;
 
-/** What should be done with the incoming socket. */
+/**
+ * What should be done with the incoming socket.
+ *
+ * <p>Be careful when using values like {@link #DISCONNECT_AT_END}, {@link #SHUTDOWN_INPUT_AT_END}
+ * and {@link #SHUTDOWN_OUTPUT_AT_END} that close a socket after a response, and where there are
+ * follow-up requests. The client is unblocked and free to continue as soon as it has received the
+ * entire response body. If and when the client makes a subsequent request using a pooled socket the
+ * server may not have had time to close the socket. The socket will be closed at an indeterminate
+ * point before or during the second request. It may be closed after client has started sending the
+ * request body. If a request body is not retryable then the client may fail the request, making
+ * client behavior non-deterministic. Add delays in the client to improve the chances that the
+ * server has closed the socket before follow up requests are made.
+ */
 public enum SocketPolicy {
 
   /**
@@ -28,6 +40,8 @@ public enum SocketPolicy {
   /**
    * Close the socket after the response. This is the default HTTP/1.0
    * behavior.
+   *
+   * <p>See {@link SocketPolicy} for reasons why this can cause test flakiness and how to avoid it.
    */
   DISCONNECT_AT_END,
 
@@ -56,17 +70,21 @@ public enum SocketPolicy {
   /**
    * Shutdown the socket input after sending the response. For testing bad
    * behavior.
+   *
+   * <p>See {@link SocketPolicy} for reasons why this can cause test flakiness and how to avoid it.
    */
   SHUTDOWN_INPUT_AT_END,
 
   /**
    * Shutdown the socket output after sending the response. For testing bad
    * behavior.
+   *
+   * <p>See {@link SocketPolicy} for reasons why this can cause test flakiness and how to avoid it.
    */
   SHUTDOWN_OUTPUT_AT_END,
 
   /**
-   * Don't response to the request but keep the socket open. For testing
+   * Don't respond to the request but keep the socket open. For testing
    * read response header timeout issue.
    */
   NO_RESPONSE

okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java

@@ -425,6 +425,11 @@ public final class URLConnectionTest {
     connection1.setReadTimeout(100);
     assertContent("This connection won't pool properly", connection1);
     assertEquals(0, server.takeRequest().getSequenceNumber());
+
+    // Give the server time to enact the socket policy if it's one that could happen after the
+    // client has received the response.
+    Thread.sleep(500);
+
     HttpURLConnection connection2 = client.open(server.getUrl("/b"));
     connection2.setReadTimeout(100);
     assertContent("This comes after a busted connection", connection2);
@@ -632,6 +637,10 @@ public final class URLConnectionTest {
     client.client().setHostnameVerifier(new RecordingHostnameVerifier());
 
     assertContent("abc", client.open(server.getUrl("/")));
+
+    // Give the server time to disconnect.
+    Thread.sleep(500);
+
     assertContent("def", client.open(server.getUrl("/")));
 
     Set<TlsVersion> tlsVersions =
@@ -1197,6 +1206,9 @@ public final class URLConnectionTest {
     // Seed the pool with a bad connection.
     assertContent("a", client.open(server.getUrl("/")));
 
+    // Give the server time to disconnect.
+    Thread.sleep(500);
+
     // This connection will need to be recovered. When it is, transparent gzip should still work!
     assertContent("b", client.open(server.getUrl("/")));
 
@@ -2577,6 +2589,9 @@ public final class URLConnectionTest {
 
     assertContent("A", client.open(server.getUrl("/a")));
 
+    // Give the server time to disconnect.
+    Thread.sleep(500);
+
     // If the request body is larger than OkHttp's replay buffer, the failure may still occur.
     byte[] requestBody = new byte[requestSize];
     new Random(0).nextBytes(requestBody);