Merge pull request #718 from rhusar/MODCLUSTER-790-v2

MODCLUSTER-790 mod_cluster listener throws NPE when deploying a context in Tomcat - refine ContainerEventHandler#add contract and implementation

Author: rhusar

container/spi/src/main/java/org/jboss/modcluster/container/ContainerEventHandler.java

@@ -42,8 +42,9 @@ public interface ContainerEventHandler {
     void shutdown();
 
     /**
-     * Indicates the deployment of a new web application. This event triggers a ENABLE-APP command for this context, if it is
-     * already started.
+     * Indicates the deployment of a new web application which is not intending to start immediately. This will issue a STOP-APP
+     * without session draining command on the proxies. In case the application will start
+     * use {@link ContainerEventHandler#start(Context)}.
      *
      * @param context the added context
      */

container/tomcat-10.0/src/main/java/org/jboss/modcluster/container/tomcat/TomcatEventHandlerAdapter.java

@@ -135,9 +135,7 @@ public void containerEvent(ContainerEvent event) {
                 ((Lifecycle) child).addLifecycleListener(this);
                 ((Container) child).addPropertyChangeListener(this);
 
-                if (this.start.get()) {
-                    this.eventHandler.add(new TomcatContext(registry, (Context) child));
-                }
+                // n.b. MODCLUSTER-790 do not ContainerEventHandler#add(..) the context here - wait for actual start(..)
             } else if (container instanceof Engine) {
                 // Deploying a host
                 container.addContainerListener(this);

container/tomcat-10.0/src/test/java/org/jboss/modcluster/container/tomcat/ContainerEventHandlerAdapterTestCase.java

@@ -169,7 +169,10 @@ public void deployWebApp() throws Exception {
 
         verify(context).addLifecycleListener(handler);
         verify(context).addPropertyChangeListener(handler);
-        verify(this.eventHandler).add(eq(catalinaContext));
+
+        LifecycleEvent lifecycleEvent = new LifecycleEvent(context, Lifecycle.AFTER_START_EVENT, context);
+        handler.lifecycleEvent(lifecycleEvent);
+        verify(this.eventHandler).start(eq(catalinaContext));
     }
 
     @Test

container/tomcat-10.1/src/main/java/org/jboss/modcluster/container/tomcat/TomcatEventHandlerAdapter.java

@@ -135,9 +135,7 @@ public void containerEvent(ContainerEvent event) {
                 ((Lifecycle) child).addLifecycleListener(this);
                 ((Container) child).addPropertyChangeListener(this);
 
-                if (this.start.get()) {
-                    this.eventHandler.add(new TomcatContext(registry, (Context) child));
-                }
+                // n.b. MODCLUSTER-790 do not ContainerEventHandler#add(..) the context here - wait for actual start(..)
             } else if (container instanceof Engine) {
                 // Deploying a host
                 container.addContainerListener(this);

container/tomcat-10.1/src/test/java/org/jboss/modcluster/container/tomcat/ContainerEventHandlerAdapterTestCase.java

@@ -169,7 +169,10 @@ public void deployWebApp() throws Exception {
 
         verify(context).addLifecycleListener(handler);
         verify(context).addPropertyChangeListener(handler);
-        verify(this.eventHandler).add(eq(catalinaContext));
+
+        LifecycleEvent lifecycleEvent = new LifecycleEvent(context, Lifecycle.AFTER_START_EVENT, context);
+        handler.lifecycleEvent(lifecycleEvent);
+        verify(this.eventHandler).start(eq(catalinaContext));
     }
 
     @Test

container/tomcat-8.5/src/main/java/org/jboss/modcluster/container/tomcat/TomcatEventHandlerAdapter.java

@@ -135,9 +135,7 @@ public void containerEvent(ContainerEvent event) {
                 ((Lifecycle) child).addLifecycleListener(this);
                 ((Container) child).addPropertyChangeListener(this);
 
-                if (this.start.get()) {
-                    this.eventHandler.add(new TomcatContext(registry, (Context) child));
-                }
+                // n.b. MODCLUSTER-790 do not ContainerEventHandler#add(..) the context here - wait for actual start(..)
             } else if (container instanceof Engine) {
                 // Deploying a host
                 container.addContainerListener(this);

container/tomcat-8.5/src/test/java/org/jboss/modcluster/container/tomcat/ContainerEventHandlerAdapterTestCase.java

@@ -169,7 +169,10 @@ public void deployWebApp() throws Exception {
 
         verify(context).addLifecycleListener(handler);
         verify(context).addPropertyChangeListener(handler);
-        verify(this.eventHandler).add(eq(catalinaContext));
+
+        LifecycleEvent lifecycleEvent = new LifecycleEvent(context, Lifecycle.AFTER_START_EVENT, context);
+        handler.lifecycleEvent(lifecycleEvent);
+        verify(this.eventHandler).start(eq(catalinaContext));
     }
 
     @Test

container/tomcat-9.0/src/main/java/org/jboss/modcluster/container/tomcat/TomcatEventHandlerAdapter.java

@@ -135,9 +135,7 @@ public void containerEvent(ContainerEvent event) {
                 ((Lifecycle) child).addLifecycleListener(this);
                 ((Container) child).addPropertyChangeListener(this);
 
-                if (this.start.get()) {
-                    this.eventHandler.add(new TomcatContext(registry, (Context) child));
-                }
+                // n.b. MODCLUSTER-790 do not ContainerEventHandler#add(..) the context here - wait for actual start(..)
             } else if (container instanceof Engine) {
                 // Deploying a host
                 container.addContainerListener(this);

container/tomcat-9.0/src/test/java/org/jboss/modcluster/container/tomcat/ContainerEventHandlerAdapterTestCase.java

@@ -169,7 +169,10 @@ public void deployWebApp() throws Exception {
 
         verify(context).addLifecycleListener(handler);
         verify(context).addPropertyChangeListener(handler);
-        verify(this.eventHandler).add(eq(catalinaContext));
+
+        LifecycleEvent lifecycleEvent = new LifecycleEvent(context, Lifecycle.AFTER_START_EVENT, context);
+        handler.lifecycleEvent(lifecycleEvent);
+        verify(this.eventHandler).start(eq(catalinaContext));
     }
 
     @Test

core/src/main/java/org/jboss/modcluster/ModClusterService.java

@@ -211,11 +211,7 @@ public void start(Server server) {
             for (Engine engine : server.getEngines()) {
                 this.config(engine);
 
-                for (Host host : engine.getHosts()) {
-                    for (Context context : host.getContexts()) {
-                        this.add(context);
-                    }
-                }
+                // n.b. MODCLUSTER-790 do not ContainerEventHandler#add(..) the context here - wait for actual start(..)
             }
         }
     }
@@ -300,12 +296,8 @@ public void add(Context context) {
         ModClusterLogger.LOGGER.addContext(context.getHost(), context);
 
         if (this.include(context) && this.established) {
-            // Send ENABLE-APP if state is started
-            if (context.isStarted()) {
-                this.enable(context);
-            } else {
-                this.stop(context);
-            }
+            // Send a STOP-APP directly, without session draining.
+            this.mcmpHandler.sendRequest(this.requestFactory.createStopRequest(context));
         }
     }