OkHttp: avoid leaked connection warning

Question

I am using OkHttp 3, and I keep getting leaked connection warnings:

WARNING: A connection to https://help.helpling.com/ was leaked. Did you forget to close a response body?
Jul 14, 2016 6:57:09 PM okhttp3.ConnectionPool pruneAndGetAllocationCount

Everytime I get a ResponseBody, I either call .string() which supposedly closes the stream for me, or I explicitly close it in a finally block, in the following way:

ResponseBody responseBody = response.body();
try (Reader responseReader = responseBody.charStream()) {
    ...
}
finally {
    responseBody.close();
}

My application makes intense use of the network, and yet that warning appears frequently. I never observed any problem caused by this presumed leak, but I would still like to understand if and what I am doing wrong.

Could anyone shed some light on this?

score 27 · Accepted Answer · answered May 05 '17 at 15:06

By upgrading to OkHttp 3.7, Eclipse started warning me of potential resource leaks. I found my problem to be in this method I wrote:

public static Response getResponse(HttpUrl url, OkHttpClient client) throws IOException {
    Builder request = new Request.Builder().url(url);
    Response response = client.newCall(request.build()).execute();
    if (!response.isSuccessful()) {
        boolean repeatRequest = handleHttpError(response);
        if (repeatRequest)
            return getResponse(url, client, etag);
        else
            throw new IOException(String.format("Cannot get successful response for url %s", url));
    }
    return response;
}

I assumed that by always calling getResponse(url, client).body().string() the stream would close automatically. But, whenever a response was unsuccessful, an exception would raise before the execution of .string(), thus the stream would remain open.

Adding an explicit close in case of unsuccessful response solved the problem.

if (!response.isSuccessful()) {
    boolean repeatRequest = handleHttpError(response);
    response.close();
}

score 15 · Answer 2 · edited Dec 15 '16 at 19:23

15

As mentioned in the other answers, you have to close the response. A slightly cleaner approach would be to declare the ResponseBody in the try block, so that it will be automatically closed.

try(ResponseBody body = ....){
....
}

edited Dec 15 '16 at 19:23

eli-k

10,898
11
40
44

answered Dec 15 '16 at 16:19

Corrigan Johnson

151
1
3

For some it might be slightly nicer to close the `Response` rather than the `ResponseBody` directly. That is, closing Response closes the underlying ResponseBody ... for my use cases closing Response works a little bit better. – Rob Bygrave Jan 09 '17 at 00:45
I could not use a try-with-resources approach, because my method needed to return the `Response` instance, to be read by another method if successful (see my own answer). – Alphaaa May 05 '17 at 15:08
1

if you use Kotlin and the new okhttpclient v4, simply consume the response using a `client.newCall(..).execute().use {response -> ... }` – Jilles van Gurp Jul 30 '19 at 15:21

sanjiv ranjan · Answer 3 · 2023-08-28T06:45:11.060

Even with success, I need to close the response for me. And body().string() did not close the connection. So explicitly we need to close the connection even if the response is 200 OK. So better put it in finally {... } block.

try{
 response = client.newCall(request).execute();
if (HttpStatus.valueOf(response.code()).is2xxSuccessful()) {

    try (ResponseBody body = response.peekBody(Long.MAX_VALUE)) {
      Identifier identifier = objectMapper.readValue(body.string(), Identifier.class);
      // response.close();
      return identifier;
    } catch (RuntimeException e) {
      // response.close();
      throw new SomeException( .....);
    }
  } else {
    // response.close();
    throw new SomeException(...);
  } ...
 } catch(RuntimeException ex){
   .......
  }
   finally { 
    if (response != null) {
    response.close();
  }
}

OkHttp: avoid leaked connection warning

3 Answers3

Linked