AsyncHttpClient / jersey-ahc-client

AsyncHttpClient transport support for Jersey
19 stars 15 forks source link

ConcurrentModificationException in cookie handling #6

Open celandro opened 10 years ago

celandro commented 10 years ago

Unfortunately this is the longest threaddump I have: at org.sonatype.spice.jersey.client.ahc.AhcClientHandler.handle(AhcClientHandler.java:137) ~[jersey-ahc-client-1.0.5.jar:na] at com.sun.jersey.api.client.Client.handle(Client.java:648) ~[jersey-client-1.12.jar:1.12] at com.sun.jersey.api.client.WebResource.handle(WebResource.java:680) ~[jersey-client-1.12.jar:1.12] at com.sun.jersey.api.client.WebResource.access$200(WebResource.java:74) ~[jersey-client-1.12.jar:1.12] at com.sun.jersey.api.client.WebResource$Builder.post(WebResource.java:558) ~[jersey-client-1.12.jar:1.12] .... removed .... Caused by: java.util.ConcurrentModificationException: null at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:859) ~[na:1.7.0_45] at java.util.ArrayList$Itr.remove(ArrayList.java:845) ~[na:1.7.0_45] at org.sonatype.spice.jersey.client.ahc.AhcClientHandler.applyResponseCookies(AhcClientHandler.java:154) ~[jersey-ahc-client-1.0.5.jar:na] at org.sonatype.spice.jersey.client.ahc.AhcClientHandler.handle(AhcClientHandler.java:125) ~[jersey-ahc-client-1.0.5.jar:na]

celandro commented 10 years ago

This ends up killing the connection pool by the way. The next exceptions are: com.sun.jersey.api.client.ClientHandlerException: java.lang.NullPointerException at org.sonatype.spice.jersey.client.ahc.AhcClientHandler.handle(AhcClientHandler.java:137) ~[jersey-ahc-client-1.0.5.jar:na] at com.sun.jersey.api.client.Client.handle(Client.java:648) ~[jersey-client-1.12.jar:1.12] at com.sun.jersey.api.client.WebResource.handle(WebResource.java:680) ~[jersey-client-1.12.jar:1.12] at com.sun.jersey.api.client.WebResource.access$200(WebResource.java:74) ~[jersey-client-1.12.jar:1.12] at com.sun.jersey.api.client.WebResource$Builder.post(WebResource.java:558) ~[jersey-client-1.12.jar:1.12] ..... removed.... Caused by: java.lang.NullPointerException: null at com.ning.http.client.cookie.CookieEncoder.encode(CookieEncoder.java:26) ~[async-http-client-1.8.10.jar:na] at com.ning.http.client.providers.netty.NettyAsyncHttpProvider.construct(NettyAsyncHttpProvider.java:821) ~[async-http-client-1.8.10.jar:na] at com.ning.http.client.providers.netty.NettyAsyncHttpProvider.buildRequest(NettyAsyncHttpProvider.java:650) ~[async-http-client-1.8.10.jar:na] at com.ning.http.client.providers.netty.NettyAsyncHttpProvider.buildNettyResponseFutureWithCachedChannel(NettyAsyncHttpProvider.java:962) ~[async-http-client-1.8.10.jar:na] at com.ning.http.client.providers.netty.NettyAsyncHttpProvider.doConnect(NettyAsyncHttpProvider.java:1014) ~[async-http-client-1.8.10.jar:na] at com.ning.http.client.providers.netty.NettyAsyncHttpProvider.execute(NettyAsyncHttpProvider.java:935) ~[async-http-client-1.8.10.jar:na] at com.ning.http.client.AsyncHttpClient.executeRequest(AsyncHttpClient.java:512) ~[async-http-client-1.8.10.jar:na] at org.sonatype.spice.jersey.client.ahc.AhcClientHandler.handle(AhcClientHandler.java:123) ~[jersey-ahc-client-1.0.5.jar:na]

Followed by com.sun.jersey.api.client.ClientHandlerException: java.io.IOException: Too many connections 10 at org.sonatype.spice.jersey.client.ahc.AhcClientHandler.handle(AhcClientHandler.java:137) ~[jersey-ahc-client-1.0.5.jar:na] at com.sun.jersey.api.client.Client.handle(Client.java:648) ~[jersey-client-1.12.jar:1.12] at com.sun.jersey.api.client.WebResource.handle(WebResource.java:680) ~[jersey-client-1.12.jar:1.12] at com.sun.jersey.api.client.WebResource.access$200(WebResource.java:74) ~[jersey-client-1.12.jar:1.12] at com.sun.jersey.api.client.WebResource$Builder.post(WebResource.java:558) ~[jersey-client-1.12.jar:1.12] ---removed--- Caused by: java.io.IOException: Too many connections 10 at com.ning.http.client.providers.netty.NettyAsyncHttpProvider.doConnect(NettyAsyncHttpProvider.java:1057) ~[async-http-client-1.8.10.jar:na] at com.ning.http.client.providers.netty.NettyAsyncHttpProvider.execute(NettyAsyncHttpProvider.java:935) ~[async-http-client-1.8.10.jar:na] at com.ning.http.client.AsyncHttpClient.executeRequest(AsyncHttpClient.java:512) ~[async-http-client-1.8.10.jar:na] at org.sonatype.spice.jersey.client.ahc.AhcClientHandler.handle(AhcClientHandler.java:123) ~[jersey-ahc-client-1.0.5.jar:na]

slandelle commented 10 years ago

Caused by: java.lang.NullPointerException: null at com.ning.http.client.cookie.CookieEncoder.encode(CookieEncoder.java:26)

If I check out CookieEncoder.java:26, you obviously have a null Cookie in the cookie list.

How you ended up with this is probably for you to figure out.

A possibility could be that you're keeping a reference to a RequestBuilder, send the Request, and then try to edit the RequestBuilder. If so, check out RequestBuilder code and you'll realize that RequestBuilder is neither threadsafe nor immutable and share its state with the Request it builds. It's just a fluent API.

celandro commented 10 years ago

We do not reuse requestbuilder in our production code.

 @Override
 public <R, T> R post(final String url, final T data, final Class<R> c) {
     final WebResource webResource               = restfulClient.resource(url);
     final WebResource.Builder requestBuilder    = webResource.getRequestBuilder();

     final Class<? extends Object> dataClass = data.getClass();
     final ClientResponse response = applyType(applyAccept(requestBuilder), dataClass).entity(data).post(ClientResponse.class);
     try {
         return getEntity(c, response);
     } finally {
         closeResponse(response);
     }
 }
celandro commented 10 years ago

PS. Please check the pull request. The new unit test throws the ConcurrentModificiationException.

I do not know what causes this exception to eventually kill the connection pool but on all 9 servers, it occurs first and eventually they all run out of connections and never recover. Some get the NullPointerException and some do not. Some have 1 ConcurrentModificiationException, some have 6

celandro commented 10 years ago

Ps. All cookie related code is only inside ahc jersey client. We do not set any cookies ourselves. They are only returned by the server, saved by AhcClientHandler and set in the request by AhcClientHandler.

The bug fix is simply to replace non-threadsafe linkedlist with a threadsafe collection.