Re: svn commit: r679208 - /ant/ivy/core/trunk/src/java/org/apache/ivy/util/url/HttpClientHandler.java

[Xavier Hanin <xa...@gmail.com>]

On Thu, Jul 24, 2008 at 12:06 AM, <ma...@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/