dcache: avoid NPE from initialization race in RestoreRequestsReceiver…

… in HttpPooMgrEngineV3

Motivation:

During system test the following NPE was observed:

06 Mar 2018 10:57:32 (httpd) [PoolManager PoolManagerGetRestoreHandlerInfo] Uncaught exception in thread httpd-0
java.lang.NullPointerException: null
        at diskCacheV111.poolManager.RestoreRequestsReceiver.messageArrived(RestoreRequestsReceiver.java:120) ~[dcache-core-4.1.0-SNAPSHOT.jar:4.1.0-SNAPSHOT]
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:1.8.0_102]
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:1.8.0_102]
        ...

The NPE stems from the fact that initialization has not occurred before the
RequestHandler in PoolManager sends the message.

This seems to be particularly the case for the HttpPoolMgrEngineV3, which is
constructed out of band (not by Spring injection), and initialized in afterStart.

Modification:

Move the initialization to the constructor, since the thread is actually
started there.

Also, initialized the webapp and frontend instances in the spring context
and remove redundant initializa call in the class,
and eliminated the unnecessary synchronized block (the cache returns
a concurrent map, so no synchronization is needed).

Result:

We should not see NPEs from this race.  Also, initialization sequence
is better.

Target: master
Request: 4.0
Require-notes: yes
Require-book: no
Acked-by: Paul

modules/dcache-frontend/src/main/java/org/dcache/restful/util/restores/RestoreCollector.java

@@ -94,7 +94,6 @@ public List<RestoreHandlerInfo> collectData() {
     public void initialize(Long timeout, TimeUnit timeUnit) {
         pnfsHandler = new PnfsHandler(pnfsStub);
         pnfsHandler.setSubject(Subjects.ROOT);
-        receiver.initialize();
         super.initialize(timeout, timeUnit);
     }
 

modules/dcache-frontend/src/main/resources/org/dcache/frontend/frontend.xml

@@ -246,7 +246,7 @@
     <description>Collects staging request info from from pool manager.</description>
     <property name="pnfsStub" ref="pnfs-stub"/>
     <property name="receiver">
-      <bean class="diskCacheV111.poolManager.RestoreRequestsReceiver">
+      <bean class="diskCacheV111.poolManager.RestoreRequestsReceiver" init-method="initialize">
         <property name="lifetime" value="${frontend.restore-requests.lifetime}"/>
         <property name="lifetimeUnit" value="${frontend.restore-requests.lifetime.unit}"/>
       </bean>

modules/dcache-webadmin/src/main/java/org/dcache/webadmin/model/dataaccess/communication/collectors/RestoreHandlerCollector.java

@@ -28,14 +28,13 @@ public Status call() throws InterruptedException {
         return Status.SUCCESS;
     }
 
-    @Override
-    public void initialize() {
-        receiver.initialize();
-        super.initialize();
-    }
-
     @Required
     public void setReceiver(RestoreRequestsReceiver receiver) {
+        /*
+         * Note that this receiver has already been initialized
+         * inside the httpd context.  It is passed via JNDI to
+         * webadmin and pulled out inside the webadmin context.
+         */
         this.receiver = receiver;
     }
 }

modules/dcache/src/main/java/diskCacheV111/poolManager/HttpPoolMgrEngineV3.java

@@ -2,6 +2,7 @@
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+
 import java.io.OutputStream;
 import java.io.PrintWriter;
 import java.text.SimpleDateFormat;
@@ -17,6 +18,15 @@
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
+import diskCacheV111.util.CacheException;
+import diskCacheV111.util.HTMLWriter;
+import diskCacheV111.util.PnfsId;
+import diskCacheV111.util.TimeoutCacheException;
+import diskCacheV111.vehicles.PnfsMapPathMessage;
+import diskCacheV111.vehicles.RestoreHandlerInfo;
+import diskCacheV111.vehicles.StorageInfo;
+import diskCacheV111.vehicles.hsmControl.HsmControlGetBfDetailsMsg;
+
 import dmg.cells.nucleus.CellCommandListener;
 import dmg.cells.nucleus.CellEndpoint;
 import dmg.cells.nucleus.CellInfo;
@@ -32,15 +42,6 @@
 import dmg.util.HttpRequest;
 import dmg.util.HttpResponseEngine;
 
-import diskCacheV111.util.CacheException;
-import diskCacheV111.util.HTMLWriter;
-import diskCacheV111.util.PnfsId;
-import diskCacheV111.util.TimeoutCacheException;
-import diskCacheV111.vehicles.PnfsMapPathMessage;
-import diskCacheV111.vehicles.RestoreHandlerInfo;
-import diskCacheV111.vehicles.StorageInfo;
-import diskCacheV111.vehicles.hsmControl.HsmControlGetBfDetailsMsg;
-
 import org.dcache.cells.CellStub;
 import org.dcache.namespace.FileAttribute;
 import org.dcache.poolmanager.Partition;
@@ -135,6 +136,7 @@ public HttpPoolMgrEngineV3(String[] argsString)
                 decodeCss(argsString[i].substring(4));
             }
         }
+        _receiver.initialize();
         _restoreCollector = new Thread(this, "restore-collector");
         _log.info("Using CSS file : {}", _cssFile);
     }
@@ -161,7 +163,6 @@ public void afterStart()
     {
         _pm.start();
         _pm.afterStart();
-        _receiver.initialize();
         _restoreCollector.start();
     }
 

modules/dcache/src/main/java/diskCacheV111/poolManager/RestoreRequestsReceiver.java

@@ -103,9 +103,7 @@ public List<RestoreHandlerInfo> getAllRequests() {
      * @return currently registered keys.
      */
     public Set<String> getPoolManagers() {
-        synchronized (restores) {
-            return restores.asMap().keySet();
-        }
+        return restores.asMap().keySet();
     }
 
     /**

modules/dcache/src/main/resources/org/dcache/services/httpd/httpd.xml

@@ -41,7 +41,8 @@
 
     <bean id="delegator" class="org.dcache.services.httpd.handlers.HandlerDelegator"/>
 
-    <bean id="restores-request-receiver" class="diskCacheV111.poolManager.RestoreRequestsReceiver">
+    <bean id="restores-request-receiver" class="diskCacheV111.poolManager.RestoreRequestsReceiver"
+          init-method="initialize">
         <property name="lifetime" value="${httpd.service.restore-requests.lifetime}"/>
         <property name="lifetimeUnit" value="${httpd.service.restore-requests.lifetime.unit}"/>
     </bean>