Bug 346419

Comment 1 Chris Dumoulin 2011-07-26 14:21:15 EDT

Is there any update on this issue?

Comment 2 Oatz 2011-09-14 16:02:16 EDT

I am also seeing this issue with Jetty 7.5.1.v20110908, the Sun JVM 1.6.0_26, and Ubuntu 10.04

Our main application uses the HttpClient without issue, but after starting to use the ProxyServlet we begin to encounter this issue. As Chris stated, the two pipe descriptors are opened after the ProxyServlet call to _client.send(exchange). 

I would expect file descriptors to be opened, however after the exchange has been completed these descriptors are not closed / removed until garbage collection occurs. This can lead to a fairly high number of "open" files in busy systems if garbage collection does not occur often. 

We've increased the limit of open files which works around the issue, but if the cleanup of these files is actually happening during garbage collection, I would take that to mean that something is not be closed correctly.

For Reference, we configure the ProxyServlet as follows

ServletContextHandler servlet = new ServletContextHandler(contexts, "/hosts");
for(Entry<String, Address> e : hostMap.entrySet())
{
    String context = "/"+e.getKey();
    Address server = e.getValue();
    ProxyServlet.Transparent proxy = 
           new ProxyServlet.Transparent(
                context,server.getHost(), server.getPort());
    servlet.addServlet(new ServletHolder(proxy), context+"/*");
}

I'll see if I can't work on a simplified testcase to reproduce the issue.

Comment 3 Greg Wilkins 2011-09-20 03:25:46 EDT

Oatz,

are you just using the ProxyServlet as a transparent proxy, or are you using the proxy feature of the HttpClient to talk to your own proxy?

Comment 4 Greg Wilkins 2011-09-20 03:54:29 EDT

Created attachment 203645 [details]
test to show it working ok

Comment 5 Greg Wilkins 2011-09-20 03:55:41 EDT

The attached test shows the number of connections growing then shrinking as you do multiple sends.  lsof verifies that all is working OK.

are you sure you have set and idle time, or max connections per destination?

Comment 6 Chris Dumoulin 2011-09-20 08:02:17 EDT

(In reply to comment #5)
> The attached test shows the number of connections growing then shrinking as you
> do multiple sends.  lsof verifies that all is working OK.
> 
> are you sure you have set and idle time, or max connections per destination?

This is how the HttpClient is being configured in our application:

httpClient = new HttpClient();
httpClient.setConnectorType(HttpClient.CONNECTOR_SELECT_CHANNEL);
httpClient.setTimeout(30000);
httpClient.setConnectTimeout(10000);
httpClient.setMaxRetries(3);
httpClient.setMaxConnectionsPerAddress(10);
httpClient.setThreadPool(new QueuedThreadPool(250));
httpClient.setMaxRedirects(20);

So, no idle timeout is being set, which should mean it defaults to 20 seconds.

Comment 7 Oatz 2011-09-20 10:53:26 EDT

Greg,

We are just using it as a transparent proxy. We use the transparent proxy to periodically retrieve a HTML status page from servers that are behind a firewall.

I hadn't set max connections or idle timeout for the transparent proxy, so I'll do that and see if it makes a difference.

Thanks,
--Oatz

Comment 8 Thomas Becker 2011-09-21 10:25:43 EDT

Created attachment 203775 [details]
proposed patch

Comment 9 Thomas Becker 2011-09-21 10:29:49 EDT

Created attachment 203777 [details]
proposed patch

I can confirm that with my Java Version there's no connection leak. However there could be if channel.socket().connect() fails after establishing a connection. 

I've added two patch files. The first adds a catch for UknownHostException surrounding channel.socket().connect() which takes care for closing the channel. With my JVM it doesn't open a connection before the host is resolved. When applying only the first patch this has the least impact on other code.

The second patch file which should be applied after the first has been will move the channel.close() call to the outer IOException catch block. From my understanding this should be pretty safe to do, so you can apply both patches. Please review.

Additionally the first patch includes a fix for HttpDestination Leak we've when a non existent host is called. Please see the diff for details.