Fixes NPE on ClientExecutionPoolSizeLowTest

pulled service access logic to nodeEngine in order not to return null from getService() move listener initilization from node constructor to start. Add start to quorum service and move listener registration there Fixes hazelcast#6853 Backport of hazelcast#6936
sancar · Dec 3, 2015 · 92bcf0f · 92bcf0f
1 parent bb874ec
commit 92bcf0f
Show file tree

Hide file tree

Showing 12 changed files with 30 additions and 61 deletions.
diff --git a/hazelcast/src/main/java/com/hazelcast/client/impl/ClientEngineImpl.java b/hazelcast/src/main/java/com/hazelcast/client/impl/ClientEngineImpl.java
@@ -506,12 +506,6 @@ private void initService(ClientRequest request) {
             }
 
             Object service = nodeEngine.getService(serviceName);
-            if (service == null) {
-                if (nodeEngine.isActive()) {
-                    throw new IllegalArgumentException("No service registered with name: " + serviceName);
-                }
-                throw new HazelcastInstanceNotActiveException();
-            }
             request.setService(service);
         }
 

diff --git a/hazelcast/src/main/java/com/hazelcast/instance/Node.java b/hazelcast/src/main/java/com/hazelcast/instance/Node.java
@@ -167,7 +167,6 @@ public Node(HazelcastInstanceImpl hazelcastInstance, Config config, NodeContext
             textCommandService = new TextCommandServiceImpl(this);
             nodeExtension.printNodeInfo(this);
             this.multicastService = createMulticastService(addressPicker);
-            initializeListeners(config);
             joiner = nodeContext.createJoiner(this);
         } catch (Throwable e) {
             try {
@@ -328,6 +327,7 @@ public void start() {
         }
         if (completelyShutdown) return;
         nodeEngine.start();
+        initializeListeners(config);
         connectionManager.start();
         if (config.getNetworkConfig().getJoin().getMulticastConfig().isEnabled()) {
             final Thread multicastServiceThread = new Thread(

diff --git a/hazelcast/src/main/java/com/hazelcast/quorum/impl/QuorumServiceImpl.java b/hazelcast/src/main/java/com/hazelcast/quorum/impl/QuorumServiceImpl.java
@@ -64,10 +64,13 @@ public QuorumServiceImpl(NodeEngineImpl nodeEngine) {
         this.nodeEngine = nodeEngine;
         this.eventService = nodeEngine.getEventService();
         initializeQuorums();
-        initializeListeners();
         this.inactive = quorums.isEmpty();
     }
 
+    public void start() {
+        initializeListeners();
+    }
+
     private void initializeQuorums() {
         for (QuorumConfig quorumConfig : nodeEngine.getConfig().getQuorumConfigs().values()) {
             QuorumImpl quorum = new QuorumImpl(quorumConfig, nodeEngine);

diff --git a/hazelcast/src/main/java/com/hazelcast/replicatedmap/impl/ReplicatedMapService.java b/hazelcast/src/main/java/com/hazelcast/replicatedmap/impl/ReplicatedMapService.java
@@ -69,17 +69,16 @@ public class ReplicatedMapService
     private final Config config;
     private final NodeEngine nodeEngine;
     private final EventService eventService;
-    private final EventRegistration eventRegistration;
 
     public ReplicatedMapService(NodeEngine nodeEngine) {
         this.nodeEngine = nodeEngine;
         this.config = nodeEngine.getConfig();
         this.eventService = nodeEngine.getEventService();
-        this.eventRegistration = eventService.registerListener(SERVICE_NAME, EVENT_TOPIC_NAME, new ReplicationListener());
     }
 
     @Override
     public void init(NodeEngine nodeEngine, Properties properties) {
+        eventService.registerListener(SERVICE_NAME, EVENT_TOPIC_NAME, new ReplicationListener());
     }
 
     @Override

diff --git a/hazelcast/src/main/java/com/hazelcast/spi/Operation.java b/hazelcast/src/main/java/com/hazelcast/spi/Operation.java
@@ -16,7 +16,6 @@
 
 package com.hazelcast.spi;
 
-import com.hazelcast.core.HazelcastException;
 import com.hazelcast.logging.ILogger;
 import com.hazelcast.logging.Logger;
 import com.hazelcast.nio.Address;
@@ -27,7 +26,6 @@
 import com.hazelcast.partition.InternalPartition;
 import com.hazelcast.quorum.QuorumException;
 import com.hazelcast.spi.exception.RetryableException;
-import com.hazelcast.spi.exception.RetryableHazelcastException;
 import com.hazelcast.spi.impl.NodeEngineImpl;
 
 import java.io.IOException;
@@ -208,14 +206,6 @@ public final <T> T getService() {
             // one might have overridden getServiceName() method...
             final String name = serviceName != null ? serviceName : getServiceName();
             service = ((NodeEngineImpl) nodeEngine).getService(name);
-            if (service == null) {
-                if (nodeEngine.isActive()) {
-                    throw new HazelcastException("Service with name '" + name + "' not found!");
-                } else {
-                    throw new RetryableHazelcastException("HazelcastInstance[" + nodeEngine.getThisAddress()
-                            + "] is not active!");
-                }
-            }
         }
         return (T) service;
     }

diff --git a/hazelcast/src/main/java/com/hazelcast/spi/impl/NodeEngineImpl.java b/hazelcast/src/main/java/com/hazelcast/spi/impl/NodeEngineImpl.java
@@ -18,11 +18,14 @@
 
 import com.hazelcast.cluster.ClusterService;
 import com.hazelcast.config.Config;
+import com.hazelcast.core.HazelcastException;
 import com.hazelcast.core.HazelcastInstance;
 import com.hazelcast.instance.GroupProperties;
 import com.hazelcast.instance.MemberImpl;
 import com.hazelcast.instance.Node;
 import com.hazelcast.internal.management.ManagementCenterService;
+import com.hazelcast.internal.storage.DataRef;
+import com.hazelcast.internal.storage.Storage;
 import com.hazelcast.logging.ILogger;
 import com.hazelcast.nio.Address;
 import com.hazelcast.nio.serialization.Data;
@@ -35,20 +38,19 @@
 import com.hazelcast.spi.PostJoinAwareService;
 import com.hazelcast.spi.ServiceInfo;
 import com.hazelcast.spi.SharedService;
-import com.hazelcast.internal.storage.DataRef;
-import com.hazelcast.internal.storage.Storage;
+import com.hazelcast.spi.exception.RetryableHazelcastException;
+import com.hazelcast.spi.impl.eventservice.InternalEventService;
+import com.hazelcast.spi.impl.eventservice.impl.EventServiceImpl;
+import com.hazelcast.spi.impl.executionservice.InternalExecutionService;
+import com.hazelcast.spi.impl.executionservice.impl.ExecutionServiceImpl;
 import com.hazelcast.spi.impl.operationservice.InternalOperationService;
 import com.hazelcast.spi.impl.operationservice.impl.OperationServiceImpl;
 import com.hazelcast.spi.impl.proxyservice.InternalProxyService;
 import com.hazelcast.spi.impl.proxyservice.impl.ProxyServiceImpl;
-import com.hazelcast.spi.impl.waitnotifyservice.InternalWaitNotifyService;
-import com.hazelcast.spi.impl.waitnotifyservice.impl.WaitNotifyServiceImpl;
-import com.hazelcast.spi.impl.eventservice.InternalEventService;
-import com.hazelcast.spi.impl.eventservice.impl.EventServiceImpl;
 import com.hazelcast.spi.impl.transceiver.PacketTransceiver;
 import com.hazelcast.spi.impl.transceiver.impl.PacketTransceiverImpl;
-import com.hazelcast.spi.impl.executionservice.InternalExecutionService;
-import com.hazelcast.spi.impl.executionservice.impl.ExecutionServiceImpl;
+import com.hazelcast.spi.impl.waitnotifyservice.InternalWaitNotifyService;
+import com.hazelcast.spi.impl.waitnotifyservice.impl.WaitNotifyServiceImpl;
 import com.hazelcast.transaction.TransactionManagerService;
 import com.hazelcast.transaction.impl.TransactionManagerServiceImpl;
 import com.hazelcast.wan.WanReplicationService;
@@ -103,6 +105,7 @@ public PacketTransceiver getPacketTransceiver() {
     public void start() {
         serviceManager.start();
         proxyService.init();
+        quorumService.start();
     }
 
     @Override
@@ -229,12 +232,22 @@ public GroupProperties getGroupProperties() {
 
     public <T> T getService(String serviceName) {
         final ServiceInfo serviceInfo = serviceManager.getServiceInfo(serviceName);
-        return serviceInfo != null ? (T) serviceInfo.getService() : null;
+        T service = serviceInfo != null ? (T) serviceInfo.getService() : null;
+        if (service == null) {
+            if (!isActive()) {
+                throw new HazelcastException("Service with name '" + serviceName + "' not found!");
+            } else {
+                throw new RetryableHazelcastException("HazelcastInstance[" + getThisAddress()
+                        + "] is not active!");
+            }
+        }
+        return service;
     }
 
     @Override
     public <T extends SharedService> T getSharedService(String serviceName) {
-        final Object service = getService(serviceName);
+        final ServiceInfo serviceInfo = serviceManager.getServiceInfo(serviceName);
+        Object service = serviceInfo != null ? serviceInfo.getService() : null;
         if (service == null) {
             return null;
         }

diff --git a/hazelcast/src/main/java/com/hazelcast/spi/impl/eventservice/impl/EventPacketProcessor.java b/hazelcast/src/main/java/com/hazelcast/spi/impl/eventservice/impl/EventPacketProcessor.java
@@ -42,9 +42,6 @@ void process(EventPacket eventPacket) {
         String serviceName = eventPacket.getServiceName();
 
         EventPublishingService<Object, Object> service = getPublishingService(serviceName);
-        if (service == null) {
-            return;
-        }
 
         Registration registration = getRegistration(eventPacket, serviceName);
         if (registration == null) {

diff --git a/hazelcast/src/main/java/com/hazelcast/spi/impl/eventservice/impl/LocalEventDispatcher.java b/hazelcast/src/main/java/com/hazelcast/spi/impl/eventservice/impl/LocalEventDispatcher.java
@@ -53,13 +53,6 @@ public TimeUnit getTimeUnit() {
     @Override
     public void run() {
         final EventPublishingService<Object, Object> service = eventService.nodeEngine.getService(serviceName);
-        if (service == null) {
-            if (eventService.nodeEngine.isActive()) {
-                throw new IllegalArgumentException("Service[" + serviceName + "] could not be found!");
-            }
-            return;
-        }
-
         service.dispatchEvent(event, listener);
     }
 

diff --git a/...ast/src/main/java/com/hazelcast/spi/impl/operationservice/impl/IsStillRunningService.java b/...ast/src/main/java/com/hazelcast/spi/impl/operationservice/impl/IsStillRunningService.java
@@ -142,10 +142,6 @@ private Operation createCheckOperation(Invocation invocation) {
      */
     public boolean isOperationExecuting(Address callerAddress, String callerUuid, String serviceName, Object identifier) {
         Object service = nodeEngine.getService(serviceName);
-        if (service == null) {
-            logger.severe("Not able to find operation execution info. Invalid service: " + serviceName);
-            return false;
-        }
         if (service instanceof ExecutionTracingService) {
             return ((ExecutionTracingService) service).isOperationExecuting(callerAddress, callerUuid, identifier);
         }

diff --git a/hazelcast/src/main/java/com/hazelcast/spi/impl/proxyservice/impl/ProxyServiceImpl.java b/hazelcast/src/main/java/com/hazelcast/spi/impl/proxyservice/impl/ProxyServiceImpl.java
@@ -149,9 +149,7 @@ public void destroyLocalDistributedObject(String serviceName, String name, boole
             registry.destroyProxy(name, fireEvent);
         }
         final RemoteService service = nodeEngine.getService(serviceName);
-        if (service != null) {
-            service.destroyDistributedObject(name);
-        }
+        service.destroyDistributedObject(name);
         String message = "DistributedObject[" + service + " -> " + name + "] has been destroyed!";
         Throwable cause = new DistributedObjectDestroyedException(message);
         nodeEngine.getWaitNotifyService().cancelWaitingOps(serviceName, name, cause);

diff --git a/hazelcast/src/main/java/com/hazelcast/transaction/impl/TransactionContextImpl.java b/hazelcast/src/main/java/com/hazelcast/transaction/impl/TransactionContextImpl.java
@@ -19,7 +19,6 @@
 import com.hazelcast.collection.impl.list.ListService;
 import com.hazelcast.collection.impl.queue.QueueService;
 import com.hazelcast.collection.impl.set.SetService;
-import com.hazelcast.core.HazelcastInstanceNotActiveException;
 import com.hazelcast.core.TransactionalList;
 import com.hazelcast.core.TransactionalMap;
 import com.hazelcast.core.TransactionalMultiMap;
@@ -127,12 +126,6 @@ public TransactionalObject getTransactionalObject(String serviceName, String nam
             obj = ((TransactionalService) service).createTransactionalObject(name, transaction);
             txnObjectMap.put(key, obj);
         } else {
-            if (service == null) {
-                if (!nodeEngine.isActive()) {
-                    throw new HazelcastInstanceNotActiveException();
-                }
-                throw new IllegalArgumentException("Unknown Service[" + serviceName + "]!");
-            }
             throw new IllegalArgumentException("Service[" + serviceName + "] is not transactional!");
         }
         return obj;

diff --git a/hazelcast/src/main/java/com/hazelcast/transaction/impl/xa/XATransactionContextImpl.java b/hazelcast/src/main/java/com/hazelcast/transaction/impl/xa/XATransactionContextImpl.java
@@ -19,7 +19,6 @@
 import com.hazelcast.collection.impl.list.ListService;
 import com.hazelcast.collection.impl.queue.QueueService;
 import com.hazelcast.collection.impl.set.SetService;
-import com.hazelcast.core.HazelcastInstanceNotActiveException;
 import com.hazelcast.core.TransactionalList;
 import com.hazelcast.core.TransactionalMap;
 import com.hazelcast.core.TransactionalMultiMap;
@@ -122,12 +121,6 @@ public TransactionalObject getTransactionalObject(String serviceName, String nam
             obj = ((TransactionalService) service).createTransactionalObject(name, transaction);
             txnObjectMap.put(key, obj);
         } else {
-            if (service == null) {
-                if (!nodeEngine.isActive()) {
-                    throw new HazelcastInstanceNotActiveException();
-                }
-                throw new IllegalArgumentException("Unknown Service[" + serviceName + "]!");
-            }
             throw new IllegalArgumentException("Service[" + serviceName + "] is not transactional!");
         }
         return obj;