diff --git a/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockWebServer.java b/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockWebServer.java index a63bbd41c..a1f02c849 100644 --- a/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockWebServer.java +++ b/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(); diff --git a/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/SocketPolicy.java b/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/SocketPolicy.java index e2d5f2848..d1c5ed536 100644 --- a/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/SocketPolicy.java +++ b/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. + * + *

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. + * + *

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. + * + *

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. + * + *

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 diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java index 330929b33..644811595 100644 --- a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java +++ b/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 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);