Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak and multiple (Http|Servlet)*Listener invokations after restart #11039

Closed
EVi1b7wO opened this issue Dec 11, 2023 · 2 comments · Fixed by #11042
Closed

Memory leak and multiple (Http|Servlet)*Listener invokations after restart #11039

EVi1b7wO opened this issue Dec 11, 2023 · 2 comments · Fixed by #11042
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@EVi1b7wO
Copy link

EVi1b7wO commented Dec 11, 2023

Jetty version(s)
10.x, 11.x, 12.x

Jetty Environment
ee8, ee9, ee10

Java version/vendor (use: java -version)
does not matter

OS type/version
does not matter

Description
Commit c59de80 introduced durable Servlets, Filters and Listeners.

In Jetty 9.4.x branch only Listeners with Source.EMBEDDED were "copied forward" on webapp restart whereas starting with Jetty 10.x doStart() of ServletHandler registers all Listeners with Source.(ANNOTATION|DESCRIPTOR) as "durable" and always retains them on webapp restarts.

The addListener Method of ServletHandler does not perform any checks and concatenates the existing "durable" list of Listeners with the passed new Listener that was found by the scan of changed files which triggered the restart of the webapp. So each restart creates n+1 instances of any Listener registered with Source.(ANNOTATION|DESCRIPTOR) and results in multiple event callbacks.

How to reproduce?

  1. Use jetty-maven-plugin with scan enabled
  2. Add any (Http|Servlet)*Listener using @weblistener annotation
  3. Add no-arg Constructor with syso print to log
  4. trigger scan and restart of Jetty and watch syso prints duplicating

Resolution
Ensure uniqueness of all Listeners by overriding hashCode and equals based on the (held) class name?

diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java                                                                                                          
index ff7a96a2f7..8c9384627d 100644
--- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java
+++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java
@@ -128,6 +128,22 @@ public class ListenerHolder extends BaseHolder<EventListener>
         }
     }

+    @Override
+    public boolean equals(Object that)
+    {
+        if (this == that)
+            return true;
+        if (!(that instanceof ListenerHolder))
+            return false;
+        return hashCode() == ((ListenerHolder)that).hashCode();
+    }
+
+    @Override
+    public int hashCode()
+    {
+        return getClassName().hashCode(); // holder class
+    }
+
     @Override
     public String toString()
     {

Validate if the Listener to be added is present within the "durable" list of Listeners.

diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java                                                                                                          
index c77bf64cb3..66101c4b7a 100644
--- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java
+++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java
@@ -797,8 +797,16 @@ public class ServletHandler extends ScopedHandler
      */
     public void addListener(ListenerHolder listener)
     {
-        if (listener != null)
-            setListeners(ArrayUtil.addToArray(getListeners(), listener, ListenerHolder.class));
+        if (listener == null)
+        {
+            return;
+        }
+
+        try (AutoLock ignored = lock())
+        {
+            if (!containsListenerHolder(listener))
+                setListeners(ArrayUtil.addToArray(getListeners(), listener, ListenerHolder.class));
+        }
     }

     public ListenerHolder[] getListeners()
@@ -1433,6 +1441,14 @@ public class ServletHandler extends ScopedHandler
         }
     }

+    protected boolean containsListenerHolder(ListenerHolder holder)
+    {
+        try (AutoLock ignored = lock())
+        {
+            return _listeners.contains(holder);
+        }
+    }
+
     protected boolean containsServletHolder(ServletHolder holder)
     {
         try (AutoLock ignored = lock())

BR

@EVi1b7wO EVi1b7wO added the Bug For general bugs on Jetty side label Dec 11, 2023
@janbartel
Copy link
Contributor

Thanks for the bug report.

For anyone following, this only happens when doing a restart with the jetty-maven-plugin. Running with standalone jetty is fine.

Investigating.

@janbartel
Copy link
Contributor

Raised PR #11042

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants