From 2a1617b5f0d4d5f350a6aae045fe90cf25d90d18 Mon Sep 17 00:00:00 2001 From: Julian Sedding Date: Tue, 4 Oct 2016 14:41:23 +0000 Subject: [PATCH] HTTPCLIENT-1782: [OSGi] List of tracked HTTPClients is mutable but not thread-safe git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1763285 13f79535-47bb-0310-9956-ffa450edef68 --- .../impl/HttpProxyConfigurationActivator.java | 50 +++++++++++-------- .../impl/OSGiCachingClientBuilderFactory.java | 10 ++-- .../osgi/impl/OSGiClientBuilderFactory.java | 10 ++-- 3 files changed, 36 insertions(+), 34 deletions(-) diff --git a/httpclient5-osgi/src/main/java/org/apache/hc/client5/http/osgi/impl/HttpProxyConfigurationActivator.java b/httpclient5-osgi/src/main/java/org/apache/hc/client5/http/osgi/impl/HttpProxyConfigurationActivator.java index 0d943eb09..152a86038 100644 --- a/httpclient5-osgi/src/main/java/org/apache/hc/client5/http/osgi/impl/HttpProxyConfigurationActivator.java +++ b/httpclient5-osgi/src/main/java/org/apache/hc/client5/http/osgi/impl/HttpProxyConfigurationActivator.java @@ -83,11 +83,7 @@ public final class HttpProxyConfigurationActivator implements BundleActivator, M private final List proxyConfigurations = new CopyOnWriteArrayList<>(); - private final List trackedHttpClients; - - public HttpProxyConfigurationActivator() { - trackedHttpClients = new WeakList(); - } + private final HttpClientTracker httpClientTracker = new HttpClientTracker(); /** * {@inheritDoc} @@ -119,7 +115,7 @@ public final class HttpProxyConfigurationActivator implements BundleActivator, M props.put(Constants.SERVICE_VENDOR, context.getBundle().getHeaders().get(Constants.BUNDLE_VENDOR)); props.put(Constants.SERVICE_DESCRIPTION, BUILDER_FACTORY_SERVICE_NAME); clientFactory = context.registerService(HttpClientBuilderFactory.class, - new OSGiClientBuilderFactory(configurator, trackedHttpClients), + new OSGiClientBuilderFactory(configurator, httpClientTracker), props); props.clear(); @@ -127,7 +123,7 @@ public final class HttpProxyConfigurationActivator implements BundleActivator, M props.put(Constants.SERVICE_VENDOR, context.getBundle().getHeaders().get(Constants.BUNDLE_VENDOR)); props.put(Constants.SERVICE_DESCRIPTION, CACHEABLE_BUILDER_FACTORY_SERVICE_NAME); cachingClientFactory = context.registerService(CachingHttpClientBuilderFactory.class, - new OSGiCachingClientBuilderFactory(configurator, trackedHttpClients), + new OSGiCachingClientBuilderFactory(configurator, httpClientTracker), props); } @@ -140,23 +136,16 @@ public final class HttpProxyConfigurationActivator implements BundleActivator, M for (final ServiceRegistration registeredConfiguration : registeredConfigurations.values()) { safeUnregister(registeredConfiguration); } + // remove all tracked services + registeredConfigurations.clear(); safeUnregister(configurator); safeUnregister(clientFactory); safeUnregister(cachingClientFactory); safeUnregister(trustedHostConfiguration); - // ensure all http clients - generated with the - are terminated - for (final CloseableHttpClient client : trackedHttpClients) { - if (null != client) { - closeQuietly(client); - } - } - - // remove all tracked services - registeredConfigurations.clear(); - // remove all tracked created clients - trackedHttpClients.clear(); + // ensure all http clients are closed + httpClientTracker.closeAll(); } /** @@ -214,11 +203,28 @@ public final class HttpProxyConfigurationActivator implements BundleActivator, M } private static void closeQuietly(final Closeable closeable) { - try { - closeable.close(); - } catch (final IOException e) { - // do nothing + if (closeable != null) { + try { + closeable.close(); + } catch (final IOException e) { + // do nothing + } } } + static class HttpClientTracker { + + private final List trackedHttpClients = new WeakList<>(); + + synchronized void track(final CloseableHttpClient client) { + trackedHttpClients.add(client); + } + + synchronized void closeAll() { + for (final CloseableHttpClient client : trackedHttpClients) { + closeQuietly(client); + } + trackedHttpClients.clear(); + } + } } diff --git a/httpclient5-osgi/src/main/java/org/apache/hc/client5/http/osgi/impl/OSGiCachingClientBuilderFactory.java b/httpclient5-osgi/src/main/java/org/apache/hc/client5/http/osgi/impl/OSGiCachingClientBuilderFactory.java index de35f03e9..42791c83a 100644 --- a/httpclient5-osgi/src/main/java/org/apache/hc/client5/http/osgi/impl/OSGiCachingClientBuilderFactory.java +++ b/httpclient5-osgi/src/main/java/org/apache/hc/client5/http/osgi/impl/OSGiCachingClientBuilderFactory.java @@ -26,8 +26,6 @@ */ package org.apache.hc.client5.http.osgi.impl; -import java.util.List; - import org.apache.hc.client5.http.impl.cache.CachingHttpClientBuilder; import org.apache.hc.client5.http.impl.sync.CloseableHttpClient; import org.apache.hc.client5.http.osgi.services.CachingHttpClientBuilderFactory; @@ -39,13 +37,13 @@ final class OSGiCachingClientBuilderFactory implements CachingHttpClientBuilderF private final HttpClientBuilderConfigurator configurator; - private List trackedHttpClients; + private final HttpProxyConfigurationActivator.HttpClientTracker httpClientTracker; OSGiCachingClientBuilderFactory( final HttpClientBuilderConfigurator configurator, - final List trackedHttpClients) { + final HttpProxyConfigurationActivator.HttpClientTracker httpClientTracker) { this.configurator = configurator; - this.trackedHttpClients = trackedHttpClients; + this.httpClientTracker = httpClientTracker; } @Override @@ -54,7 +52,7 @@ final class OSGiCachingClientBuilderFactory implements CachingHttpClientBuilderF @Override public CloseableHttpClient build() { final CloseableHttpClient client = super.build(); - trackedHttpClients.add(client); + httpClientTracker.track(client); return client; } }); diff --git a/httpclient5-osgi/src/main/java/org/apache/hc/client5/http/osgi/impl/OSGiClientBuilderFactory.java b/httpclient5-osgi/src/main/java/org/apache/hc/client5/http/osgi/impl/OSGiClientBuilderFactory.java index 3b9e1f5a9..f3b2e695e 100644 --- a/httpclient5-osgi/src/main/java/org/apache/hc/client5/http/osgi/impl/OSGiClientBuilderFactory.java +++ b/httpclient5-osgi/src/main/java/org/apache/hc/client5/http/osgi/impl/OSGiClientBuilderFactory.java @@ -26,8 +26,6 @@ */ package org.apache.hc.client5.http.osgi.impl; -import java.util.List; - import org.apache.hc.client5.http.impl.sync.CloseableHttpClient; import org.apache.hc.client5.http.impl.sync.HttpClientBuilder; import org.apache.hc.client5.http.osgi.services.HttpClientBuilderFactory; @@ -39,13 +37,13 @@ final class OSGiClientBuilderFactory implements HttpClientBuilderFactory { private final HttpClientBuilderConfigurator configurator; - private final List trackedHttpClients; + private final HttpProxyConfigurationActivator.HttpClientTracker httpClientTracker; OSGiClientBuilderFactory( final HttpClientBuilderConfigurator configurator, - final List trackedHttpClients) { + final HttpProxyConfigurationActivator.HttpClientTracker httpClientTracker) { this.configurator = configurator; - this.trackedHttpClients = trackedHttpClients; + this.httpClientTracker = httpClientTracker; } @Override @@ -54,7 +52,7 @@ final class OSGiClientBuilderFactory implements HttpClientBuilderFactory { @Override public CloseableHttpClient build() { final CloseableHttpClient client = super.build(); - trackedHttpClients.add(client); + httpClientTracker.track(client); return client; } });