get() seems to be leaking OkHttp connections in 5.11

[scholzj]

Discussed in #3694

Describe the bug

It seems that in some cases, when the get() method is called, the OkHttp responses are not closed properly. As a result, the application logs might be full of following messages:

Dec 27, 2021 3:11:34 PM okhttp3.internal.platform.Platform log
WARNING: A connection to https://10.96.0.1/ was leaked. Did you forget to close a response body? To see where this was allocated, set the OkHttpClient logger level to FINE: Logger.getLogger(OkHttpClient.class.getName()).setLevel(Level.FINE);
Dec 27, 2021 3:11:34 PM okhttp3.internal.platform.Platform log
WARNING: A connection to https://10.96.0.1/ was leaked. Did you forget to close a response body? To see where this was allocated, set the OkHttpClient logger level to FINE: Logger.getLogger(OkHttpClient.class.getName()).setLevel(Level.FINE);
Dec 27, 2021 3:11:34 PM okhttp3.internal.platform.Platform log
WARNING: A connection to https://10.96.0.1/ was leaked. Did you forget to close a response body? To see where this was allocated, set the OkHttpClient logger level to FINE: Logger.getLogger(OkHttpClient.class.getName()).setLevel(Level.FINE);

This happens for example when MixedOperation.inNamespace(...).withName(...).get() or NonNamespaceOperation.withName(...).get() is called. The full stack where the response is not closed looks for example like this:

Dec 27, 2021 5:11:21 PM okhttp3.internal.platform.Platform log
WARNING: A connection to https://172.30.0.1/ was leaked. Did you forget to close a response body?
java.lang.Throwable: response.body().close()
	at okhttp3.internal.platform.Platform.getStackTraceForCloseable(Platform.java:148)
	at okhttp3.RealCall.captureCallStackTrace(RealCall.java:116)
	at okhttp3.RealCall.execute(RealCall.java:88)
	at io.fabric8.kubernetes.client.okhttp.OkHttpClientImpl.send(OkHttpClientImpl.java:138)
	at io.fabric8.kubernetes.client.dsl.base.OperationSupport.retryWithExponentialBackoff(OperationSupport.java:575)
	at io.fabric8.kubernetes.client.dsl.base.OperationSupport.handleResponse(OperationSupport.java:554)
	at io.fabric8.kubernetes.client.dsl.base.OperationSupport.handleResponse(OperationSupport.java:519)
	at io.fabric8.kubernetes.client.dsl.base.OperationSupport.handleGet(OperationSupport.java:488)
	at io.fabric8.kubernetes.client.dsl.base.OperationSupport.handleGet(OperationSupport.java:458)
	at io.fabric8.kubernetes.client.dsl.base.BaseOperation.handleGet(BaseOperation.java:696)
	at io.fabric8.kubernetes.client.dsl.base.BaseOperation.getMandatory(BaseOperation.java:182)
	at io.fabric8.kubernetes.client.dsl.base.BaseOperation.get(BaseOperation.java:149)
	at io.fabric8.kubernetes.client.dsl.base.BaseOperation.get(BaseOperation.java:83)
	at io.strimzi.operator.common.operator.resource.AbstractResourceOperator.lambda$reconcile$0(AbstractResourceOperator.java:109)
	at io.vertx.core.impl.ContextImpl.lambda$null$0(ContextImpl.java:159)
	at io.vertx.core.impl.AbstractContext.dispatch(AbstractContext.java:100)
	at io.vertx.core.impl.ContextImpl.lambda$executeBlocking$1(ContextImpl.java:157)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:829)

Where the io.strimzi.operator.common.operator.resource.AbstractResourceOperator class is what calls get() on line 109.

This was not happening in 5.10.1.

For more details, see the discussion in #3694.

Fabric8 Kubernetes Client version

other (please specify in additional context)

Steps to reproduce

Following reproducer can be used to reproduce the issue also without Strimzi:

final KubernetesClient client = new DefaultKubernetesClient();
java.util.logging.Logger.getLogger(OkHttpClient.class.getName()).setLevel(Level.FINE);

Runnable getter = () -> {
    while (true) {
        client.rbac().clusterRoles().withName("some-name").get();

        try {
            Thread.sleep(1000L);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
};

Thread t1 = new Thread(getter);
t1.start();

Just run it for few minutes with 5.11.0 or 5.11.1 and you should see the error.

Expected behavior

All responses are closed and no warnings about leaked connections are raised by OkHttp.

Runtime

Kubernetes (vanilla)

Kubernetes API Server version

1.22.3@latest

Environment

Linux

Fabric8 Kubernetes Client Logs

No response

Additional context

Fabric8 versions impacted by this: 5.11.0 and 5.11.1. This relates to the discussion #3694

[walnut-tom]

just revert the https://github.com/fabric8io/kubernetes-client/pull/3549 removed 4 line code(the finally clause), it's works ok

OperationSupport.java

protected <T> T handleResponse(OkHttpClient client, Request.Builder requestBuilder, Class<T> type, Map<String, String> parameters) throws ExecutionException, InterruptedException, IOException {
    VersionUsageUtils.log(this.resourceT, this.apiGroupVersion);
    Request request = requestBuilder.build();
    Response response = retryWithExponentialBackoff(client, request);
    try (ResponseBody body = response.body()) {
      assertResponseCode(request, response);
      if (type != null) {
        try (InputStream bodyInputStream = body.byteStream()) {
          return Serialization.unmarshal(bodyInputStream, type, parameters);
        }
      } else {
        return null;
      }
    } catch (Exception e) {
      if (e instanceof KubernetesClientException) {
        throw e;
      }
      throw requestException(request, e);
    } finally {
      if(response != null && response.body() != null) {
        response.body().close();
      }
    }
  }

[rohanKanojia]

@walnut-tom : Could you please review #3699 ?

[walnut-tom]

@walnut-tom : Could you please review #3699 ?

no error