Skip to content

Commit

Permalink
Fix connection leaks on failed web socket upgrades.
Browse files Browse the repository at this point in the history
Closes: #4258
  • Loading branch information
squarejesse committed Nov 7, 2018
1 parent 550afce commit 62f2f82
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@

import java.io.EOFException;
import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.ProtocolException;
import java.net.SocketTimeoutException;
import java.util.Arrays;
import java.util.Collections;
import java.util.Random;
import java.util.concurrent.CountDownLatch;
Expand All @@ -27,6 +29,7 @@
import java.util.logging.Logger;
import okhttp3.Interceptor;
import okhttp3.OkHttpClient;
import okhttp3.Protocol;
import okhttp3.RecordingEventListener;
import okhttp3.RecordingHostnameVerifier;
import okhttp3.Request;
Expand Down Expand Up @@ -695,6 +698,35 @@ public final class WebSocketHttpTest {
clientListener.assertTextMessage("Hello, WebSockets!");
}

/**
* We had a bug where web socket connections were leaked if the HTTP connection upgrade was not
* successful. This test confirms that connections are released back to the connection pool!
* https://github.com/square/okhttp/issues/4258
*/
@Test public void webSocketConnectionIsReleased() throws Exception {
// This test assumes HTTP/1.1 pooling semantics.
client = client.newBuilder()
.protocols(Arrays.asList(Protocol.HTTP_1_1))
.build();

webServer.enqueue(new MockResponse()
.setResponseCode(HttpURLConnection.HTTP_NOT_FOUND)
.setBody("not found!"));
webServer.enqueue(new MockResponse());

newWebSocket();
clientListener.assertFailure();

Request regularRequest = new Request.Builder()
.url(webServer.url("/"))
.build();
Response response = client.newCall(regularRequest).execute();
response.close();

assertEquals(0, webServer.takeRequest().getSequenceNumber());
assertEquals(1, webServer.takeRequest().getSequenceNumber());
}

private MockResponse upgradeResponse(RecordedRequest request) {
String key = request.getHeader("Sec-WebSocket-Key");
return new MockResponse()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,7 @@ public StreamAllocation streamAllocation() {
}

if (followUp == null) {
if (!forWebSocket) {
streamAllocation.release();
}
streamAllocation.release();
return response;
}

Expand Down

0 comments on commit 62f2f82

Please sign in to comment.