I agree, The goal is to have one single instance of HttpClient here to be able to reuse the connections. At the moment, the HttpClientHandler isn't a singleton, so the easiest way for me was to make it static.
But maybe we can wait to see if the change solves the problem. If it doesn't, we can revert it, but if it solves to problem, we can refactor it to some cleaner solution. Maarten ----- Original Message ---- From: Xavier Hanin <[EMAIL PROTECTED]> To: dev@ant.apache.org Sent: Thursday, July 24, 2008 9:08:32 AM Subject: Re: svn commit: r679208 - /ant/ivy/core/trunk/src/java/org/apache/ivy/util/url/HttpClientHandler.java On Thu, Jul 24, 2008 at 12:06 AM, <[EMAIL PROTECTED]> wrote: > Author: maartenc > Date: Wed Jul 23 15:06:48 2008 > New Revision: 679208 > > URL: http://svn.apache.org/viewvc?rev=679208&view=rev > Log: > Attempt to fix connection leak reported in IVY-854 by caching the > HttpClient instance. > > Modified: > > ant/ivy/core/trunk/src/java/org/apache/ivy/util/url/HttpClientHandler.java > > Modified: > ant/ivy/core/trunk/src/java/org/apache/ivy/util/url/HttpClientHandler.java > URL: > http://svn.apache.org/viewvc/ant/ivy/core/trunk/src/java/org/apache/ivy/util/url/HttpClientHandler.java?rev=679208&r1=679207&r2=679208&view=diff > > ============================================================================== > --- > ant/ivy/core/trunk/src/java/org/apache/ivy/util/url/HttpClientHandler.java > (original) > +++ > ant/ivy/core/trunk/src/java/org/apache/ivy/util/url/HttpClientHandler.java > Wed Jul 23 15:06:48 2008 > @@ -33,6 +33,7 @@ > import org.apache.commons.httpclient.HttpException; > import org.apache.commons.httpclient.HttpMethodBase; > import org.apache.commons.httpclient.HttpStatus; > +import org.apache.commons.httpclient.MultiThreadedHttpConnectionManager; > import org.apache.commons.httpclient.UsernamePasswordCredentials; > import org.apache.commons.httpclient.auth.AuthPolicy; > import org.apache.commons.httpclient.methods.GetMethod; > @@ -61,6 +62,8 @@ > private String proxyPasswd = null; > > private HttpClientHelper httpClientHelper; > + > + private static HttpClient httpClient; I'm not sure reusing always the same (static) instance of httpClient is safe. In the context of IvyDE, we will have the same instance of httpClient whatever the number of settings we use. I know we have other similar problems about using static stuff which shouldn't be, but I dislike introducing new ones. WDYT? Xavier > > public HttpClientHandler() { > configureProxy(); > @@ -204,28 +207,39 @@ > } > > private HttpClient getClient(URL url) { > - HttpClient client = new HttpClient(); > - > - List authPrefs = new ArrayList(2); > - authPrefs.add(AuthPolicy.DIGEST); > - authPrefs.add(AuthPolicy.BASIC); > - // Exclude the NTLM authentication scheme because it is not > supported by this class > - client.getParams().setParameter(AuthPolicy.AUTH_SCHEME_PRIORITY, > authPrefs); > - > - if (useProxy()) { > - client.getHostConfiguration().setProxy(proxyHost, proxyPort); > - if (useProxyAuthentication()) { > - client.getState().setProxyCredentials(proxyRealm, > proxyHost, > - new UsernamePasswordCredentials(proxyUserName, > proxyPasswd)); > + if (httpClient == null) { > + final MultiThreadedHttpConnectionManager connManager = new > MultiThreadedHttpConnectionManager(); > + httpClient = new HttpClient(connManager); > + > + Runtime.getRuntime().addShutdownHook(new Thread(new Runnable() > { > + public void run() { > + connManager.shutdown(); > + } > + })); > + > + List authPrefs = new ArrayList(2); > + authPrefs.add(AuthPolicy.DIGEST); > + authPrefs.add(AuthPolicy.BASIC); > + // Exclude the NTLM authentication scheme because it is not > supported by this class > + > httpClient.getParams().setParameter(AuthPolicy.AUTH_SCHEME_PRIORITY, > authPrefs); > + > + if (useProxy()) { > + httpClient.getHostConfiguration().setProxy(proxyHost, > proxyPort); > + if (useProxyAuthentication()) { > + httpClient.getState().setProxyCredentials(proxyRealm, > proxyHost, > + new UsernamePasswordCredentials(proxyUserName, > proxyPasswd)); > + } > } > } > + > Credentials c = getCredentials(url); > if (c != null) { > Message.debug("found credentials for " + url + ": " + c); > - client.getState().setCredentials(c.getRealm(), c.getHost(), > + httpClient.getState().setCredentials(c.getRealm(), > c.getHost(), > new UsernamePasswordCredentials(c.getUserName(), > c.getPasswd())); > } > - return client; > + > + return httpClient; > } > > private boolean useProxy() { > @@ -243,7 +257,7 @@ > private boolean useProxyAuthentication() { > return (proxyUserName != null && proxyUserName.trim().length() > > 0); > } > - > + > private static final class GETInputStream extends InputStream { > private InputStream is; > > > > -- Xavier Hanin - Independent Java Consultant http://xhab.blogspot.com/ http://ant.apache.org/ivy/ http://www.xoocode.org/ --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
