Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

closeIdleConnectionsAfterEachResponse closes the response before consuming is possible #633

Closed
mdzhigarov opened this issue Jan 20, 2016 · 30 comments

Comments

@mdzhigarov
Copy link

Im using the following configuration to reproduce the issue:

RestAssured.config = RestAssured.config().connectionConfig(new ConnectionConfig().closeIdleConnectionsAfterEachResponse());
RequestSpecification request = given().relaxedHTTPSValidation()
.contentType(Constants.XML_CONTENT_TYPE)
.auth().basic("usr",
"pass");
request .contentType("application/json");
request.accept("application/json");
request.body(...);
request.when().post(restURL).then().extract().response().asString();

The asString() call throws exception:
java.net.SocketException: Socket is closed
at sun.security.ssl.SSLSocketImpl.checkEOF(SSLSocketImpl.java:1520)
at sun.security.ssl.AppInputStream.read(AppInputStream.java:95)
at org.apache.http.impl.io.AbstractSessionInputBuffer.fillBuffer(AbstractSessionInputBuffer.java:160)
at org.apache.http.impl.io.SocketInputBuffer.fillBuffer(SocketInputBuffer.java:84)
at org.apache.http.impl.io.AbstractSessionInputBuffer.readLine(AbstractSessionInputBuffer.java:273)
at org.apache.http.impl.conn.LoggingSessionInputBuffer.readLine(LoggingSessionInputBuffer.java:116)
at org.apache.http.impl.io.ChunkedInputStream.getChunkSize(ChunkedInputStream.java:240)
at org.apache.http.impl.io.ChunkedInputStream.nextChunk(ChunkedInputStream.java:206)
at org.apache.http.impl.io.ChunkedInputStream.read(ChunkedInputStream.java:169)
at java.util.zip.InflaterInputStream.fill(InflaterInputStream.java:238)
at java.util.zip.InflaterInputStream.read(InflaterInputStream.java:158)
at java.util.zip.GZIPInputStream.read(GZIPInputStream.java:117)
at org.apache.http.conn.EofSensorInputStream.read(EofSensorInputStream.java:137)
at org.apache.http.conn.EofSensorInputStream$read.call(Unknown Source)
at com.jayway.restassured.internal.RestAssuredResponseOptionsGroovyImpl.convertStreamToByteArray(RestAssuredResponseOptionsGroovyImpl.groovy:430)
at sun.reflect.GeneratedMethodAccessor158.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:497)
at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:93)
at org.codehaus.groovy.runtime.callsite.StaticMetaMethodSite$StaticMetaMethodSiteNoUnwrapNoCoerce.invoke(StaticMetaMethodSite.java:151)
at org.codehaus.groovy.runtime.callsite.StaticMetaMethodSite.callStatic(StaticMetaMethodSite.java:102)
at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callStatic(AbstractCallSite.java:206)
at com.jayway.restassured.internal.RestAssuredResponseOptionsGroovyImpl.charsetToString(RestAssuredResponseOptionsGroovyImpl.groovy:482)
at com.jayway.restassured.internal.RestAssuredResponseOptionsGroovyImpl$charsetToString$3.callCurrent(Unknown Source)
at com.jayway.restassured.internal.RestAssuredResponseOptionsGroovyImpl.asString(RestAssuredResponseOptionsGroovyImpl.groovy:170)
at com.jayway.restassured.internal.RestAssuredResponseOptionsGroovyImpl.asString(RestAssuredResponseOptionsGroovyImpl.groovy:166)
at com.jayway.restassured.internal.RestAssuredResponseOptionsImpl.asString(RestAssuredResponseOptionsImpl.java:194)

This happens only with json. application/xml is deserialized correctly.

Seems like the socket is closed prematurely.
Please investigate

@johanhaleby
Copy link
Collaborator

Hmm sounds very strange, never seen anything like it. It's extremely hard for me to know what's going on unless I can reproduce it. I don't suppose you can give me any guidance on how I can do that?

@DarinVenkov
Copy link

Hi,

I made a dummy project https://github.com/DarinVenkov/rest-assured-dummy with a simple service deployed on a fresh Apache Tomcat 8.0.30 and a client with JUnit tests to try to reproduce the use case.
The project is bare bones and doesn't give the exact same stack trace but it is giving an org.apache.http.ConnectionClosedException exception after trying to get the response body when using the closeIdleConnectionsAfterEachResponse configuration. And I'm getting it both with xml and json. There are also tests without the configuration and they don't fail.

How can we consume the response bodies after the connections have been closed. It's necessary for the connections to close and the response to be consumable even after that. If there is another way to accomplish that or if there isn't please look into the issue.

@johanhaleby
Copy link
Collaborator

Thanks a lot for taking the time to create this project, I'll try to look into it. I only have 15 minutes now until spouse and kid gets home so I'm not sure I'll make it today :)

@johanhaleby
Copy link
Collaborator

How do I actually run the service? I can't do mvn jetty:run? I'm not using Eclipse (I'm an intellij user)

@johanhaleby
Copy link
Collaborator

I added the Maven jetty plugin and then it worked.

@johanhaleby
Copy link
Collaborator

Hmm so when I change the URL to be http://localhost:8080/rest/dummy/person (which is the URL that works when I start it with Jetty) only ClosedConnectionsTestJson fails. But it fails with:

org.apache.http.MalformedChunkCodingException: CRLF expected at end of chunk

    at org.apache.http.impl.io.ChunkedInputStream.getChunkSize(ChunkedInputStream.java:255)
    at org.apache.http.impl.io.ChunkedInputStream.nextChunk(ChunkedInputStream.java:227)
    at org.apache.http.impl.io.ChunkedInputStream.read(ChunkedInputStream.java:186)
    at org.apache.http.conn.EofSensorInputStream.read(EofSensorInputStream.java:137)
    at org.apache.http.conn.EofSensorInputStream$read.call(Unknown Source)
    at com.jayway.restassured.internal.RestAssuredResponseOptionsGroovyImpl.convertStreamToByteArray(RestAssuredResponseOptionsGroovyImpl.groovy:430)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:497)
    at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:93)
    at org.codehaus.groovy.runtime.callsite.StaticMetaMethodSite$StaticMetaMethodSiteNoUnwrapNoCoerce.invoke(StaticMetaMethodSite.java:151)
    at org.codehaus.groovy.runtime.callsite.StaticMetaMethodSite.callStatic(StaticMetaMethodSite.java:102)
    at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallStatic(CallSiteArray.java:56)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callStatic(AbstractCallSite.java:194)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callStatic(AbstractCallSite.java:206)

@johanhaleby
Copy link
Collaborator

If I remove request.config(RestAssured.config().connectionConfig(new ConnectionConfig().closeIdleConnectionsAfterEachResponse())); then it also works. Why would you like to configure RA to closeIdleConnectionsAfterEachResponse?

@johanhaleby
Copy link
Collaborator

Also if I change to request.config(RestAssured.config().connectionConfig(new ConnectionConfig().closeIdleConnectionsAfterEachResponseAfter(10, TimeUnit.MILLISECONDS))); it works. It seems like the server is returning chunked content and it's not safe to use closeIdleConnectionsAfterEachResponse when using chunked content (see javadoc). Perhaps the javadocs should be improved though.. The documentation also says this:

However if you're downloading (especially large amounts of) chunked data you must not close connections after each response.

I'm going to close the issue for now. We can continue the discussion if you like and tell me if you think I should reopen the issue.

@mdzhigarov
Copy link
Author

@johanhaleby I think you underestimated the severity of that issue. I take full responsibility for not explaining the problem better :)

This bug is REAL and very critical.

You see, a real world continuous integration environment often would contain several hundred or even thousands of test cases. These test cases will usually run in a complicated VM/docker infrastructure, on build machines coordinated by Jenkins.
Such complex infrastructures are actually very common in the enterprise world.
And in order for such infrastructure to operate smoothly and stable, it MUST NOT have any resource leaks. And this is exactly why I opened this issue - rest-assured is introducing socket leackages into our environment.

The problem is that after each test case:
(request.when().post(restURL).then().extract().response()....)
... a new socket is being open. This socket is then never closed, unless one explicitly specifies the closeIdleConnectionsAfterEachResponse() configuration. You can see that for yourself - just open TCPView or any other TCP sniffer and invoke several hundred requests using rest-assured. You will notice hundreds of sockets being opened all at once and then never closed.

So we decided to use the closeIdleConnectionsAfterEachResponse() config in order to properly clean up the connection state after each test.
We identified three problems with that:
1] If you set this option globally:
RestAssured.config = config().connectionConfig(new ConnectionConfig().closeIdleConnectionsAfterEachResponse());

... it doesn't even work. The sockets are not getting closed after each response, even though the configuration is there. Again, you can see this for yourself using TCPView.

2] If you set this option "per configuration" like that:
request.config(RestAssured.config().connectionConfig(new ConnectionConfig().closeIdleConnectionsAfterEachResponse()))

... it does close the sockets, but if the accept header contains "application/json" then the sockets are being closed prematurely, before the last chunk of information arrives.

3] The closeIdleConnectionsAfterEachResponseAfter(...) method does not work at all.
The time out is not even used for anything and the sockets again get closed prematurely.

So we end up in a situation where either all sockets will never get closed, or they will all get closed prematurely. As I said, in my opinion this makes the rest-assured api unusable. At least for us this is a blocker.

So please have a look into this.

@oxygen0211
Copy link
Contributor

Hi, I'm the lead maintainer of the integration test platform @mdzhigarov talked about. I took a look at where the connections should probably be closed and why they aren't. Here's my theory:

RequestSpecificationImpl is using HttpEntity.consumeContent, which is deprecated and doesn't provide a default implementation, in the finally block of doRequest. I think since the responses we are getting should contain InputStreamEntities which are relying on the default implementation.

For testing if I'm right, I'll replace the call to getEntity with a different stream consumptions. I'll submit a PR once I've got a working solution.

@johanhaleby
Copy link
Collaborator

Sorry for not getting back on this earlier, I've been so busy :(

@oxygen0211 If you can find a way to get this working it would be great. Really appreciate your effort in tracking this down.

@oxygen0211
Copy link
Contributor

@johanhaleby sure, no problem. We have a big interest in getting this up and running for testing rest services for our products.

If my theory is right, it should just be switching to EntityUtils for consuming the entity. I'm just dealing with some hickups in the groovy parts of the maven build at the moment before being able to verify this.

@johanhaleby
Copy link
Collaborator

If you're using intellij you should just be able to open the pom.xml and the project should build. If you're using Eclipse I have no clue :)

@oxygen0211
Copy link
Contributor

I'm using eclipse, I guess that makes it a bit complicated, but in the meantime, tests are running, so we'll see if my fix (and what I could build) is sufficient :-)

@oxygen0211
Copy link
Contributor

With PR #639, I couldn't reproduce our problems anymore

@johanhaleby
Copy link
Collaborator

I've merged the pull request and published a new snapshot. Please try it out by depending on version 2.8.1-SNAPSHOT after having added the following maven repo:

<repositories>
        <repository>
            <id>sonatype</id>
            <url>https://oss.sonatype.org/content/repositories/snapshots/</url>
            <snapshots />
        </repository>
</repositories>

Thanks for your help

@oxygen0211
Copy link
Contributor

Sorry for letting you wait, but I didn't get to test the SNAPSHOT earlier. Looks good so far, couldn't reproduce the problem. Is there a plan when the 2.8.1 release will be available?

@johanhaleby
Copy link
Collaborator

Probably in a few weeks. There are some other issues I'd like to sort out first.

@johanhaleby
Copy link
Collaborator

It should be safe to use the snapshot though.

@oxygen0211
Copy link
Contributor

Thanks for the info. Using snapshots in production is prohibited by our company policy to make sure we won't suddenly get changed behavior due to an updated snapshot, so I'm thinking more of building an internal version including the change until you have released it officially.

Anyways, thanks for your help and the fast handling of the issue. I consider this as fixed (at least until I'm proven wrong :-)).

@oxygen0211
Copy link
Contributor

Little update on this one:

Although at first it looked like the problem was solved by my PR, we are still experiencing hanging connections with a growing base of tests. My guess is that there are similar places in the code to what I fixed that are not working smoothely with the refactorings introduced to httpclient a while ago.

Unfortunately, these problems are blockers for using rest-assured in our test env. We had to find another solution, so from our side, there is no urgency to fix this. However, I would love to see these problems solved since rest-assured provides much value for API developers and I think that it would find use in other places of our company if reliable enough.

@johanhaleby
Copy link
Collaborator

Ok that's bad news :( Unfortunately it'll be extremely hard for me to track this down since I don't have a setup like yours. If this is to be solved anytime soon I will need your help of tracking it down.

@mdzhigarov
Copy link
Author

@johanhaleby
This is actually quite easy to reproduce. We already provided for you a dummy project:
https://github.com/DarinVenkov/rest-assured-dummy

I just updated the project a bit - it now includes jetty so you'll be able to start the service in a few seconds. The junit tests in the client are also updated.

What you need to do:

  • run mvn jetty:run in the service project in order to start the service
  • run mvn test in the client project

You will notice two junit tests in the client project - one is using content-type application/xml, the other - application/json.
The latter is failing with:

org.apache.http.MalformedChunkCodingException: CRLF expected at end of chunk

at org.apache.http.impl.io.ChunkedInputStream.getChunkSize(ChunkedInputStream.java:255)
at org.apache.http.impl.io.ChunkedInputStream.nextChunk(ChunkedInputStream.java:227)
at org.apache.http.impl.io.ChunkedInputStream.read(ChunkedInputStream.java:186)
at org.apache.http.conn.EofSensorInputStream.read(EofSensorInputStream.java:137)
at org.apache.http.conn.EofSensorInputStream$read.call(Unknown Source)
...

IMHO this is a bug which needs addressing.
We need the closeIdleConnectionsAfterEachResponse option because we make a lot of consecutive requests with small response bodies. If we omit this option, suddenly hundreds of inet sockets are being opened in a small window of time. These sockets then are never closed (they stay in CLOSE_WAIT state. See: http://blogs.technet.com/b/janelewis/archive/2010/03/09/explaining-close-wait.aspx for more info)

Setting closeIdleConnectionsAfterEachResponseAfter(1000, TimeUnit.MILLISECONDS)) in the configuration doesn't solve the issue either.

@chenhm
Copy link

chenhm commented Mar 29, 2016

@mdzhigarov you can reuse the http clients, not need close them.

System.setProperty("http.maxConnections","100");
config = config().httpClient(httpClientConfig().reuseHttpClientInstance().httpClientFactory(
    () -> new SystemDefaultHttpClient()
));

@gpor0
Copy link

gpor0 commented Nov 14, 2016

I made a fork with close() method added to Response object as a possible workaround.
close() method calls connectionManager.shutdown() under the hood.

https://github.com/gpor89/rest-assured

@set321go
Copy link

@johanhaleby Can this issue be reopened this still seems to be an ongoing problem.

These are my global settings

    RestAssured.baseURI = "http://" + System.getProperty("API_HOST");
    RestAssured.port = Integer.parseInt(System.getProperty("API_PORT", "8080"));
    RestAssured.config = RestAssured.config()
        .connectionConfig(connectionConfig().closeIdleConnectionsAfterEachResponse())
        .httpClient(httpClientConfig().reuseHttpClientInstance());

I make two POST's in the same test one after the other, their ordering is not important I have switched them around and still get the same error. I also tried putting Thread.sleep(1000) between them but no change.

    // Create Extension
    given()
        .contentType(ContentType.JSON)
        .accept(ContentType.JSON)
        .body(ext)
        .when()
        .post("/extension");

    // Create Customer
    String customerId = given()
        .contentType(ContentType.JSON)
        .accept(ContentType.JSON)
        .body(cust)
        .when()
        .post("/")
        .then()
        .statusCode(201)
        .extract()
        .header("Location");

I have the wire logging turned on and i can verify that the data is created on the server for both requests. It looks like the connection is closed but not released.

    00:19:35.298 [Test worker] DEBUG org.apache.http.impl.client.DefaultHttpClient - Connection can be kept alive indefinitely
    00:19:35.313 [Test worker] DEBUG io.restassured.internal.RequestSpecificationImpl$RestAssuredHttpBuilder - Parsing response as: application/json
    00:19:35.313 [Test worker] DEBUG io.restassured.internal.RequestSpecificationImpl$RestAssuredHttpBuilder - Parsed data to instance of: class org.apache.http.conn.EofSensorInputStream
    00:19:35.338 [Test worker] DEBUG org.apache.http.impl.conn.DefaultClientConnection - Connection 0.0.0.0:54724<->127.0.0.1:9080 closed
    00:19:36.359 [Test worker] DEBUG org.apache.http.impl.conn.BasicClientConnectionManager - Get connection for route {}->http://localhost:9080

Here is the top of stacktrace but it doesn't line up with master

    java.lang.IllegalStateException: Invalid use of BasicClientConnManager: connection still allocated.
    Make sure to release the connection before allocating another one.
        at org.apache.http.util.Asserts.check(Asserts.java:34)
        at org.apache.http.impl.conn.BasicClientConnectionManager.getConnection(BasicClientConnectionManager.java:166)
        at org.apache.http.impl.conn.BasicClientConnectionManager$1.getConnection(BasicClientConnectionManager.java:148)
        at org.apache.http.impl.client.DefaultRequestDirector.execute(DefaultRequestDirector.java:424)
        at org.apache.http.impl.client.AbstractHttpClient.doExecute(AbstractHttpClient.java:884)
        at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:82)
        at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:55)
        at org.apache.http.client.HttpClient$execute$0.call(Unknown Source)
        at io.restassured.internal.RequestSpecificationImpl$RestAssuredHttpBuilder.doRequest(RequestSpecificationImpl.groovy:2035)

I'm having trouble with the build, getting the itests to pass. This is the error i'm getting from the itests on master

Failed tests:   using_statically_configured_proxy_defined_using_string_uri_without_port(io.restassured.itest.java.ProxyITest): 

@hindsholm
Copy link

hindsholm commented Aug 17, 2018

I have also run into this issue because our tests open so many connections that we started running out of ports on our workstations. I solved it by adding a way to explicitly close the connection as previously shown by @gpor89 . So my suggestion for a (temporary) solution would be to simply add a close() method to the Response.

@tottawang
Copy link

tottawang commented Apr 5, 2020

but why we need to close the connection, the connection can be reused for subsequent requests. The right fix is to reuse your http client as well as connection and make sure that you close the stream after socket read is done for every request.

  1. reuse http client, you won't have lots of CLOSE_WAIT tcp connections
  2. reuse connection with pooling manager, you won't have lots of TIME_WAIT tcp connections.

@geyuqiu
Copy link

geyuqiu commented Aug 29, 2021

RestAssured.reset(); solved for me ...

@Varga-Bence
Copy link

Hi everyone!

Just a little hint on the issue:
After so many days of trying, we found out, that a problem is with void endpoints, no matter what is your http and connection config related to closing idle connections. If you call endpoints with void return type, the connections won't be closed, the buffer will slowly become full, and the following exception will be thrown: java.net.SocketException: No buffer space available (maximum connections reached?): connect
(The default buffer size is 8192, which can be changed by creating a new org.apache.http.config.ConnectionConfig instance with given buffer size and setting it in RestAssured config. However, it is not a solution.)
In our case, we modified the API endpoint to return the id of the inserted object instead of void, so the problem is gone. We use reuseHttpClientInstance() config in order not to have growing number of TIME_WAIT connections, which would also cause the above exception to be thrown.

I hope this observation will help to solve the problem in RestAssured soon, or at least will help you to do a workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests