On Thu, Jul 24, 2008 at 9:25 AM, Maarten Coene <[EMAIL PROTECTED]> wrote:
> 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. It sounds like a good approach! Xavier > > > 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] > > -- Xavier Hanin - Independent Java Consultant http://xhab.blogspot.com/ http://ant.apache.org/ivy/ http://www.xoocode.org/
