From 0163d2d0c707c8c791f972f53d322adbb9e68c20 Mon Sep 17 00:00:00 2001 From: "mai.jh" Date: Wed, 15 Jul 2020 14:12:44 +0800 Subject: [PATCH] fix: #3324, Move http client close method to NamingProxy.shutdown() (#3333) --- .../naming/net/NamingHttpClientManager.java | 35 +++++++-- .../nacos/client/naming/net/NamingProxy.java | 5 +- .../nacos/client/security/SecurityProxy.java | 2 +- .../common/http/HttpClientBeanHolder.java | 78 ++++++++----------- .../alibaba/nacos/test/naming/NamingBase.java | 2 +- 5 files changed, 66 insertions(+), 56 deletions(-) diff --git a/client/src/main/java/com/alibaba/nacos/client/naming/net/NamingHttpClientManager.java b/client/src/main/java/com/alibaba/nacos/client/naming/net/NamingHttpClientManager.java index a900c2b8c6e..b66ee09e123 100644 --- a/client/src/main/java/com/alibaba/nacos/client/naming/net/NamingHttpClientManager.java +++ b/client/src/main/java/com/alibaba/nacos/client/naming/net/NamingHttpClientManager.java @@ -16,20 +16,24 @@ package com.alibaba.nacos.client.naming.net; -import com.alibaba.nacos.client.utils.LogUtils; +import com.alibaba.nacos.api.exception.NacosException; import com.alibaba.nacos.common.http.AbstractHttpClientFactory; import com.alibaba.nacos.common.http.HttpClientBeanHolder; import com.alibaba.nacos.common.http.HttpClientConfig; import com.alibaba.nacos.common.http.HttpClientFactory; import com.alibaba.nacos.common.http.client.NacosRestTemplate; +import com.alibaba.nacos.common.lifecycle.Closeable; +import com.alibaba.nacos.common.utils.ExceptionUtil; import org.slf4j.Logger; +import static com.alibaba.nacos.client.utils.LogUtils.NAMING_LOGGER; + /** * http Manager. * * @author mai.jh */ -public class NamingHttpClientManager { +public class NamingHttpClientManager implements Closeable { private static final int READ_TIME_OUT_MILLIS = Integer .getInteger("com.alibaba.nacos.client.naming.rtimeout", 50000); @@ -42,17 +46,38 @@ public class NamingHttpClientManager { private static final HttpClientFactory HTTP_CLIENT_FACTORY = new NamingHttpClientFactory(); - public static String getPrefix() { + private static class NamingHttpClientManagerInstance { + + private static final NamingHttpClientManager INSTANCE = new NamingHttpClientManager(); + } + + public static NamingHttpClientManager getInstance() { + return NamingHttpClientManagerInstance.INSTANCE; + } + + public String getPrefix() { if (ENABLE_HTTPS) { return "https://"; } return "http://"; } - public static NacosRestTemplate getNacosRestTemplate() { + public NacosRestTemplate getNacosRestTemplate() { return HttpClientBeanHolder.getNacosRestTemplate(HTTP_CLIENT_FACTORY); } + @Override + public void shutdown() throws NacosException { + NAMING_LOGGER.warn("[NamingHttpClientManager] Start destroying NacosRestTemplate"); + try { + HttpClientBeanHolder.shutdownNacostSyncRest(HTTP_CLIENT_FACTORY.getClass().getName()); + } catch (Exception ex) { + NAMING_LOGGER.error("[NamingHttpClientManager] An exception occurred when the HTTP client was closed : {}", + ExceptionUtil.getStackTrace(ex)); + } + NAMING_LOGGER.warn("[NamingHttpClientManager] Destruction of the end"); + } + private static class NamingHttpClientFactory extends AbstractHttpClientFactory { @Override @@ -63,7 +88,7 @@ protected HttpClientConfig buildHttpClientConfig() { @Override protected Logger assignLogger() { - return LogUtils.NAMING_LOGGER; + return NAMING_LOGGER; } } } diff --git a/client/src/main/java/com/alibaba/nacos/client/naming/net/NamingProxy.java b/client/src/main/java/com/alibaba/nacos/client/naming/net/NamingProxy.java index f9f685e45af..ba85bde72a2 100644 --- a/client/src/main/java/com/alibaba/nacos/client/naming/net/NamingProxy.java +++ b/client/src/main/java/com/alibaba/nacos/client/naming/net/NamingProxy.java @@ -79,7 +79,7 @@ */ public class NamingProxy implements Closeable { - private final NacosRestTemplate nacosRestTemplate = NamingHttpClientManager.getNacosRestTemplate(); + private final NacosRestTemplate nacosRestTemplate = NamingHttpClientManager.getInstance().getNacosRestTemplate(); private static final int DEFAULT_SERVER_PORT = 8848; @@ -591,7 +591,7 @@ public String callServer(String api, Map params, Map SINGLETON_REST = new HashMap(10); - private static final Map SINGLETON_ASYNC_REST = new HashMap(10); - - private static final AtomicBoolean ALREADY_SHUTDOWN = new AtomicBoolean(false); - - static { - ThreadUtils.addShutdownHook(new Runnable() { - @Override - public void run() { - shutdown(); - } - }); - } + private static final Map SINGLETON_ASYNC_REST = new HashMap( + 10); public static NacosRestTemplate getNacosRestTemplate(Logger logger) { return getNacosRestTemplate(new DefaultHttpClientFactory(logger)); @@ -99,40 +82,41 @@ public static NacosAsyncRestTemplate getNacosAsyncRestTemplate(HttpClientFactory } /** - * Shutdown http client holder and close all template. + * Shutdown http client holder and close remove template. + * + * @param className HttpClientFactory implement class name + * @throws Exception ex */ - public static void shutdown() { - if (!ALREADY_SHUTDOWN.compareAndSet(false, true)) { - return; - } - LOGGER.warn("[HttpClientBeanFactory] Start destroying NacosRestTemplate"); - try { - nacostRestTemplateShutdown(); - nacosAsyncRestTemplateShutdown(); - } catch (Exception ex) { - LOGGER.error("[HttpClientBeanFactory] An exception occurred when the HTTP client was closed : {}", - ExceptionUtil.getStackTrace(ex)); - } - LOGGER.warn("[HttpClientBeanFactory] Destruction of the end"); + public static void shutdown(String className) throws Exception { + shutdownNacostSyncRest(className); + shutdownNacosAsyncRest(className); } - private static void nacostRestTemplateShutdown() throws Exception { - if (!SINGLETON_REST.isEmpty()) { - Collection nacosRestTemplates = SINGLETON_REST.values(); - for (NacosRestTemplate nacosRestTemplate : nacosRestTemplates) { - nacosRestTemplate.close(); - } - SINGLETON_REST.clear(); + /** + * Shutdown sync http client holder and remove template. + * + * @param className HttpClientFactory implement class name + * @throws Exception ex + */ + public static void shutdownNacostSyncRest(String className) throws Exception { + final NacosRestTemplate nacosRestTemplate = SINGLETON_REST.get(className); + if (nacosRestTemplate != null) { + nacosRestTemplate.close(); + SINGLETON_REST.remove(className); } } - private static void nacosAsyncRestTemplateShutdown() throws Exception { - if (!SINGLETON_ASYNC_REST.isEmpty()) { - Collection nacosAsyncRestTemplates = SINGLETON_ASYNC_REST.values(); - for (NacosAsyncRestTemplate nacosAsyncRestTemplate : nacosAsyncRestTemplates) { - nacosAsyncRestTemplate.close(); - } - SINGLETON_ASYNC_REST.clear(); + /** + * Shutdown async http client holder and remove template. + * + * @param className HttpClientFactory implement class name + * @throws Exception ex + */ + public static void shutdownNacosAsyncRest(String className) throws Exception { + final NacosAsyncRestTemplate nacosAsyncRestTemplate = SINGLETON_ASYNC_REST.get(className); + if (nacosAsyncRestTemplate != null) { + nacosAsyncRestTemplate.close(); + SINGLETON_ASYNC_REST.remove(className); } } } diff --git a/test/src/test/java/com/alibaba/nacos/test/naming/NamingBase.java b/test/src/test/java/com/alibaba/nacos/test/naming/NamingBase.java index c06ccb8aafd..df7bd20e6c5 100644 --- a/test/src/test/java/com/alibaba/nacos/test/naming/NamingBase.java +++ b/test/src/test/java/com/alibaba/nacos/test/naming/NamingBase.java @@ -33,7 +33,7 @@ */ public class NamingBase extends HttpClient4Test { - private static final NacosRestTemplate nacosRestTemplate = NamingHttpClientManager.getNacosRestTemplate(); + private static final NacosRestTemplate nacosRestTemplate = NamingHttpClientManager.getInstance().getNacosRestTemplate(); public static final String TEST_DOM_1 = "nacos.test.1"; public static final String TEST_IP_4_DOM_1 = "127.0.0.1";