ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Maarten Coene <maarten_co...@yahoo.com>
Subject Re: svn commit: r679208 - /ant/ivy/core/trunk/src/java/org/apache/ivy/util/url/HttpClientHandler.java
Date Thu, 24 Jul 2008 07:25:05 GMT
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 <xavier.hanin@gmail.com>
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, <maartenc@apache.org> 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: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Mime
View raw message