Properly close the Apache response so that connections can be reused

[agherardi]

This is a clone of https://github.com/jersey/jersey/pull/3752

[agherardi]

Not sure why the ip-validation check is failing. I entered my email address on https://accounts.eclipse.org/users/agherardi/eca, the tool says "There is a valid ECA on file for my-email-address".

Also, not sure why the CI build fails, when running travis.sh -Ptravis_e2e all test pass on my desktop.

[chkal]

@agherardi You have to "sign off" your commits using the -s option of Git. This will add a special line to your commit message. See the Eclipse Foundation documentation for details:

https://wiki.eclipse.org/Development_Resources/Contributing_via_Git#Signing_off_on_a_commit

[jansupol]

Fixes #3629

[agherardi]

Hi - Any updates?

[jansupol]

Sorry it takes long. I want to verify regarding the #3629.

[isopropylcyanide]

Thanks @agherardi for fixing this

2 years down the line and because of a missing response.close() in previous versions of Jersey in a finally block, the pooled http connection was never released.

Our leased http connections never were returned to the pool because response wasn't being explicitly read into entities in some cases.

Our request threads talked to a database and had written few rows (REPEATABLE_READ). As a result, the JDBI handle was never closed (thread was in PARKED state due to no more available connections in the pool) and exclusive locks were held as the request was neither rolled back nor committed but suspended.

We have finally figured out the root cause. Thank you.

[jansupol]

Please understand that this also depends on the Apache HTTP Client version. See also a followup discussion.

[chriskessel]

I'm unable to upgrade past Jersey 2.25.1 since 2.26 and beyond are breaking changes with Dropwizard 1.3 (some hk2/guice thing, I think). Yes, I want to upgrade Dropwizard, but that's a large task.

Is there a hack/workaround I can use to fix this without upgrading? Some way of closing connections/responses or setting timeout properties?

I'm hitting this pretty regularly in a ForkJoinPool that does 10 parallel requests and eventually the http client just stops working and timing out every time on my Future.get().

[jansupol]

Since 2.26 you need additional Jersey dependency:
org.glassfish.jersey.inject:jersey-hk2
Does it help?

[chriskessel]

Unfortunately, no. I can't upgrade to 2.26 because there's a breaking dependency with adding hk2. It has a race condition with Dropwizard 1.x (1.3) related to something with a module to use Guice. I spent most of the last 2 days trying to get Jersey (2.32 I think) working, chasing never ending dependency issues, but could never get past some service provider race condition at startup. Nor could I get around a conflicting dependency between hibernate-validators and hibernate-validators-spi (I think it's those two), declaring the same package, that came as a result of those attempted upgrades 👍

Hence, my wondering if there is a workaround or a way to use the 2.25 such that this connection hang/leak won't occur?

[isopropylcyanide]

@chriskessel If you ensure that the inbound JAX-RS response is always closed, you'll never face a leak. Because the response isn't closed, the HTTP connection you've leased from the pool will not be returned back as the response associated with it hasn't been used yet. If Apache devs return the connection blindly and you try to read the response later, it's going to be a problem (response has been already closed)

. If you consume entities from only 2xx responses, response will be properly closed since reading off of the inputstream closes the entity as well.

Two options

• Always close the JAX-RS response (in a finally block)
• Always read the entity from a response.

The first one should be preferred. Use an Instrumented Http Connection Pool. Plot metrics of each pool. Monitor the pool leased allocation rate against the leased total rate. You should be good.

[chriskessel]

Always close the JAX-RS response (in a finally block)

Unfortunately, I already do that and it still has the issue. I use a little lombok delegate wrapper I created.

@Value
class ClosableResponse extends Response implements AutoCloseable {
        @Delegate
        Response original;
    }

Then all usages look like:

  try (ClosableResponse response = new ClosableResponse(httpClient.target(uri)
                .request(MediaType.APPLICATION_XML)
                .get(Response.class))) {
            ...do stuff here...
        }

I suppose I could add an entity read just before the close out of paranoia?

[jansupol]

Response is AutoClosable in JAX-RS 2.1 (Jersey 2.26+ dependency). Apache connector supports ClientProperties.READ_TIMEOUT and ClientProperties.CONNECT_TIMEOUT.

[chriskessel]

@jansupol Good to know! Though, as I noted, I can't move past 2.25.1 without upgrading other libraries (including the main framework we use).

I have added those timeouts, maybe that will help.