Jetty 9.4.x 2903 delay listener instantiation (#2930)

* Issue #2903 Ensure listeners not instantiated for quickstart gen

Signed-off-by: Jan Bartel <janb@webtide.com>

jetty-annotations/src/main/java/org/eclipse/jetty/annotations/WebListenerAnnotation.java

@@ -82,11 +82,9 @@ public class WebListenerAnnotation extends DiscoveredAnnotation
                 MetaData metaData = _context.getMetaData();           
                 if (metaData.getOrigin(clazz.getName()+".listener") == Origin.NotSet)
                 {     
-                    java.util.EventListener listener = (java.util.EventListener)_context.getServletContext().createInstance(clazz); 
                     ListenerHolder h = _context.getServletHandler().newListenerHolder(new Source(Source.Origin.ANNOTATION, clazz.getName()));
-                    h.setListener(listener);
+                    h.setHeldClass(clazz);
                     _context.getServletHandler().addListener(h);
-                    _context.addEventListener(listener);
                 }
             }
             else

jetty-maven-plugin/pom.xml

@@ -185,6 +185,26 @@
       <artifactId>jetty-server</artifactId>
       <version>${project.version}</version>
     </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty</groupId>
+      <artifactId>jetty-servlet</artifactId>
+      <version>${project.version}</version>
+    </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty</groupId>
+      <artifactId>jetty-client</artifactId>
+      <version>${project.version}</version>
+    </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty</groupId>
+      <artifactId>jetty-http</artifactId>
+      <version>${project.version}</version>
+    </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty</groupId>
+      <artifactId>jetty-io</artifactId>
+      <version>${project.version}</version>
+    </dependency>
     <dependency>
       <groupId>org.eclipse.jetty</groupId>
       <artifactId>jetty-jmx</artifactId>

jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/QuickStartDescriptorGenerator.java

@@ -51,6 +51,7 @@ import org.eclipse.jetty.security.authentication.FormAuthenticator;
 import org.eclipse.jetty.servlet.ErrorPageErrorHandler;
 import org.eclipse.jetty.servlet.FilterHolder;
 import org.eclipse.jetty.servlet.FilterMapping;
+import org.eclipse.jetty.servlet.ListenerHolder;
 import org.eclipse.jetty.servlet.ServletContextHandler.JspConfig;
 import org.eclipse.jetty.servlet.ServletHandler;
 import org.eclipse.jetty.servlet.ServletHolder;
@@ -176,10 +177,10 @@ public class QuickStartDescriptorGenerator
             .tag("param-value",_webApp.getInitParameter(p))
             .closeTag();
 
-        if (_webApp.getEventListeners() != null)
+        if (_webApp.getServletHandler().getListeners() != null)
-            for (EventListener e : _webApp.getEventListeners())
+            for (ListenerHolder e : _webApp.getServletHandler().getListeners())
-                out.openTag("listener",origin(md,e.getClass().getCanonicalName() + ".listener"))
+                out.openTag("listener",origin(md,e.getClassName() + ".listener"))
-                .tag("listener-class",e.getClass().getCanonicalName())
+                .tag("listener-class",e.getClassName())
                 .closeTag();
 
         ServletHandler servlets = _webApp.getServletHandler();

jetty-quickstart/src/test/java/org/eclipse/jetty/quickstart/TestQuickStart.java

@@ -24,6 +24,7 @@ import static org.junit.jupiter.api.Assertions.*;
 import java.io.File;
 
 import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.servlet.ListenerHolder;
 import org.eclipse.jetty.servlet.ServletHolder;
 import org.eclipse.jetty.toolchain.test.FS;
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
@@ -70,7 +71,9 @@ public class TestQuickStart
         fooHolder.setServlet(new FooServlet());
         fooHolder.setName("foo");
         quickstart.getServletHandler().addServlet(fooHolder);
-        quickstart.addEventListener(new FooContextListener());      
+        ListenerHolder lholder = new ListenerHolder();
+        lholder.setListener(new FooContextListener());
+        quickstart.getServletHandler().addListener(lholder);
         server.setHandler(quickstart);
         server.start();
         server.stop();

jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java

@@ -21,54 +21,112 @@ package org.eclipse.jetty.servlet;
 
 import java.util.EventListener;
 
+import javax.servlet.ServletContext;
+import javax.servlet.ServletException;
+
 /**
  * ListenerHolder
  *
- * Specialization of AbstractHolder for servlet listeners. This
+ * Specialization of BaseHolder for servlet listeners. This
  * allows us to record where the listener originated - web.xml,
  * annotation, api etc.
  */
 public class ListenerHolder extends BaseHolder<EventListener>
 {
     private EventListener _listener;
+    private boolean _initialized = false;
     
 
+    public ListenerHolder ()
+    {
+        this (Source.EMBEDDED);
+    }
+    
     public ListenerHolder(Source source)
     {
         super(source);
     }
    
    
-    public void setListener(EventListener listener)
-    {
-        _listener = listener;
-        setClassName(listener.getClass().getName());
-        setHeldClass(listener.getClass());
-        _extInstance=true;
-    }
-
     public EventListener getListener()
     {
         return _listener;
     }
 
+    /**
+     * Set an explicit instance. In this case,
+     * just like ServletHolder and FilterHolder,
+     * the listener will not be introspected for
+     * annotations like Resource etc.
+     * 
+     * @param listener
+     */
+    public void setListener (EventListener listener)
+    {
+        _listener = listener;
+        _extInstance=true;
+        setHeldClass(_listener.getClass());
+        setClassName(_listener.getClass().getName());
+    }
+
+
+    public void initialize (ServletContext context) throws Exception
+    {
+        if (!_initialized)
+        {
+            initialize();
+
+            if (_listener == null)
+            {
+                //create an instance of the listener and decorate it
+                try
+                {                    
+                    _listener = (context instanceof ServletContextHandler.Context)
+                            ?((ServletContextHandler.Context)context).createListener(getHeldClass())
+                            :getHeldClass().getDeclaredConstructor().newInstance();
+
+                }
+                catch (ServletException se)
+                {
+                    Throwable cause = se.getRootCause();
+                    if (cause instanceof InstantiationException)
+                        throw (InstantiationException)cause;
+                    if (cause instanceof IllegalAccessException)
+                        throw (IllegalAccessException)cause;
+                    throw se;
+                }
+            }
+            _initialized = true;
+        }
+    }
+
 
     @Override
     public void doStart() throws Exception
     {
-        //Listeners always have an instance eagerly created, it cannot be deferred to the doStart method
-        if (_listener == null)
-            throw new IllegalStateException("No listener instance");
-        
         super.doStart();
+        if (!java.util.EventListener.class.isAssignableFrom(_class))
+        {
+            String msg = _class+" is not a java.util.EventListener";
+            super.stop();
+            throw new IllegalStateException(msg);
+        }
     }
 
 
+
+    @Override
+    public void doStop() throws Exception
+    {
+        super.doStop();
+        if (!_extInstance)
+            _listener = null;
+        _initialized = false;
+    }
+
     @Override
     public String toString()
     {
-        return super.toString()+(_listener == null?"":": "+getClassName());
+        return super.toString()+": "+getClassName();
     } 
-    
-    
 }

jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java

@@ -356,13 +356,16 @@ public class ServletContextHandler extends ContextHandler
 
         if (_servletHandler != null)
         {
-            // Call decorators on all holders, and also on any EventListeners before
+            //Ensure listener instances are created, added to ContextHandler
-            // decorators are called on any other classes (like servlets and filters)
             if(_servletHandler.getListeners() != null)
             {
                 for (ListenerHolder holder:_servletHandler.getListeners())
                 {             
-                    _objFactory.decorate(holder.getListener());
+                    holder.start();
+                    //we need to pass in the context because the ServletHandler has not
+                    //yet got a reference to the ServletContext (happens in super.startContext)
+                    holder.initialize(_scontext);
+                    addEventListener(holder.getListener());
                 }
             }
         }

jetty-webapp/src/main/java/org/eclipse/jetty/webapp/StandardDescriptorProcessor.java

@@ -1912,16 +1912,10 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor
 
                 ((WebDescriptor)descriptor).addClassName(className);
                 
-                Class<? extends EventListener> listenerClass = (Class<? extends EventListener>)context.loadClass(className);
+                ListenerHolder h = context.getServletHandler().newListenerHolder(new Source (Source.Origin.DESCRIPTOR, descriptor.getResource().toString()));
-                listener = newListenerInstance(context,listenerClass, descriptor);
+                h.setClassName(className);
-                if (!(listener instanceof EventListener))
+                context.getServletHandler().addListener(h);
-                {
-                    LOG.warn("Not an EventListener: " + listener);
-                    return;
-                }
-                context.addEventListener(listener);
                 context.getMetaData().setOrigin(className+".listener", descriptor);
-
             }
         }
         catch (Exception e)
@@ -1960,14 +1954,4 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor
 
         ((ConstraintAware)context.getSecurityHandler()).setDenyUncoveredHttpMethods(true);
     }
-
-    public EventListener newListenerInstance(WebAppContext context,Class<? extends EventListener> clazz, Descriptor descriptor) throws Exception
-    {
-        ListenerHolder h = context.getServletHandler().newListenerHolder(new Source (Source.Origin.DESCRIPTOR, descriptor.getResource().toString()));
-        EventListener l = context.getServletContext().createInstance(clazz);
-        h.setListener(l);
-        context.getServletHandler().addListener(h);
-        return l;
-
-    }
 }