geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From u..@apache.org
Subject [geode] 01/03: GEODE-3774: moved service loader initialization around to avoid double-check locking
Date Tue, 17 Oct 2017 17:50:28 GMT
This is an automated email from the ASF dual-hosted git repository.

udo pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 397daecb4627292afacc20449a4ecb31e8c019c6
Author: kohlmu-pivotal <ukohlmeyer@pivotal.io>
AuthorDate: Wed Oct 11 12:21:02 2017 -0700

    GEODE-3774: moved service loader initialization around to avoid double-check locking
---
 .../tier/sockets/ClientProtocolServiceLoader.java  | 32 +++++++++++-----
 .../tier/sockets/ServerConnectionFactory.java      | 43 ++++++----------------
 .../cache/tier/sockets/TcpServerFactory.java       |  3 +-
 3 files changed, 34 insertions(+), 44 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientProtocolServiceLoader.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientProtocolServiceLoader.java
index 7e1e3f5..5051c05 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientProtocolServiceLoader.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientProtocolServiceLoader.java
@@ -15,26 +15,38 @@
 
 package org.apache.geode.internal.cache.tier.sockets;
 
-import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
 import java.util.ServiceLoader;
 
 public class ClientProtocolServiceLoader {
-  public ClientProtocolService loadService() {
-    ServiceLoader<ClientProtocolService> loader = ServiceLoader.load(ClientProtocolService.class);
-    Iterator<ClientProtocolService> iterator = loader.iterator();
+  private final List<ClientProtocolService> clientProtocolServices;
 
-    if (!iterator.hasNext()) {
+  public ClientProtocolServiceLoader() {
+    clientProtocolServices = initializeProtocolServices();
+  }
+
+  private static List<ClientProtocolService> initializeProtocolServices() {
+    List<ClientProtocolService> resultList = new LinkedList<>();
+    for (ClientProtocolService clientProtocolService : ServiceLoader
+        .load(ClientProtocolService.class)) {
+      resultList.add(clientProtocolService);
+    }
+
+
+    return resultList;
+  }
+
+  public ClientProtocolService lookupService() {
+    if (clientProtocolServices.isEmpty()) {
       throw new ServiceLoadingFailureException(
           "There is no ClientProtocolService implementation found in JVM");
     }
 
-    ClientProtocolService service = iterator.next();
-
-    if (iterator.hasNext()) {
+    if (clientProtocolServices.size() > 1) {
       throw new ServiceLoadingFailureException(
           "There is more than one ClientProtocolService implementation found in JVM; aborting");
     }
-
-    return service;
+    return clientProtocolServices.get(0);
   }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
index 8c80710..1897563 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
@@ -34,43 +34,21 @@ import org.apache.geode.security.internal.server.Authenticator;
  * Creates instances of ServerConnection based on the connection mode provided.
  */
 public class ServerConnectionFactory {
+  private final ClientProtocolServiceLoader clientProtocolServiceLoader;
   private volatile ClientProtocolService clientProtocolService;
-  private Map<String, Class<? extends Authenticator>> authenticators = null;
+  private Map<String, Class<? extends Authenticator>> authenticators;
 
-  public ServerConnectionFactory() {}
-
-  private synchronized void initializeAuthenticatorsMap() {
-    if (authenticators != null) {
-      return;
-    }
-    HashMap<String, Class<? extends Authenticator>> tmp = new HashMap<>();
+  public ServerConnectionFactory() {
 
+    authenticators = new HashMap<>();
     ServiceLoader<Authenticator> loader = ServiceLoader.load(Authenticator.class);
     for (Authenticator streamAuthenticator : loader) {
-      tmp.put(streamAuthenticator.implementationID(), streamAuthenticator.getClass());
-    }
-
-    authenticators = tmp;
-  }
-
-  private synchronized ClientProtocolService initializeClientProtocolService(
-      StatisticsFactory statisticsFactory, String statisticsName) {
-    if (clientProtocolService != null) {
-      return clientProtocolService;
+      authenticators.put(streamAuthenticator.implementationID(), streamAuthenticator.getClass());
     }
-
-    // use temp to make sure we publish properly.
-    ClientProtocolService tmp = new ClientProtocolServiceLoader().loadService();
-    tmp.initializeStatistics(statisticsName, statisticsFactory);
-
-    clientProtocolService = tmp;
-    return clientProtocolService;
+    clientProtocolServiceLoader = new ClientProtocolServiceLoader();
   }
 
   private Authenticator findStreamAuthenticator(String implementationID) {
-    if (authenticators == null) {
-      initializeAuthenticatorsMap();
-    }
     Class<? extends Authenticator> streamAuthenticatorClass = authenticators.get(implementationID);
     if (streamAuthenticatorClass == null) {
       throw new ServiceLoadingFailureException(
@@ -86,10 +64,11 @@ public class ServerConnectionFactory {
     }
   }
 
-  private ClientProtocolService getOrCreateClientProtocolService(
+  private synchronized ClientProtocolService getClientProtocolService(
       StatisticsFactory statisticsFactory, String serverName) {
     if (clientProtocolService == null) {
-      return initializeClientProtocolService(statisticsFactory, serverName);
+      clientProtocolService = clientProtocolServiceLoader.lookupService();
+      clientProtocolService.initializeStatistics(serverName, statisticsFactory);
     }
     return clientProtocolService;
   }
@@ -98,7 +77,7 @@ public class ServerConnectionFactory {
       CachedRegionHelper helper, CacheServerStats stats, int hsTimeout, int socketBufferSize,
       String communicationModeStr, byte communicationMode, Acceptor acceptor,
       SecurityService securityService) throws IOException {
-    if (communicationMode == ProtobufClientServerProtocol.getModeNumber()) {
+    if (ProtobufClientServerProtocol.getModeNumber() == communicationMode) {
       if (!Boolean.getBoolean("geode.feature-protobuf-protocol")) {
         throw new IOException("Server received unknown communication mode: " + communicationMode);
       } else {
@@ -124,7 +103,7 @@ public class ServerConnectionFactory {
       String communicationModeStr, byte communicationMode, Acceptor acceptor,
       SecurityService securityService, String authenticationMode) {
     ClientProtocolService service =
-        getOrCreateClientProtocolService(cache.getDistributedSystem(), acceptor.getServerName());
+        getClientProtocolService(cache.getDistributedSystem(), acceptor.getServerName());
 
     ClientProtocolProcessor processor = service.createProcessorForCache(cache,
         findStreamAuthenticator(authenticationMode), securityService);
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/TcpServerFactory.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/TcpServerFactory.java
index bf442f8..f8e903f 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/TcpServerFactory.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/TcpServerFactory.java
@@ -20,7 +20,6 @@ import java.util.Properties;
 
 import org.apache.logging.log4j.Logger;
 
-import org.apache.geode.cache.IncompatibleVersionException;
 import org.apache.geode.distributed.internal.DistributionConfigImpl;
 import org.apache.geode.distributed.internal.InternalLocator;
 import org.apache.geode.distributed.internal.PoolStatHelper;
@@ -35,7 +34,7 @@ public class TcpServerFactory {
   public TcpServerFactory() {
     ClientProtocolService service = null;
     try {
-      service = new ClientProtocolServiceLoader().loadService();
+      service = new ClientProtocolServiceLoader().lookupService();
     } catch (ServiceLoadingFailureException ex) {
       logger.warn("Could not load client protocol", ex);
     }

-- 
To stop receiving notification emails like this one, please contact
"commits@geode.apache.org" <commits@geode.apache.org>.

Mime
View raw message