From 6084705725d6d36e708fe462b9350dcf39006a57 Mon Sep 17 00:00:00 2001 From: gregw Date: Mon, 23 Sep 2024 11:21:52 +1000 Subject: [PATCH] Fix #12296 by partial revert of "Fixes #12289 - Improve ConcurrentPool concurrency. (#12290)" This reverts/modified parts of commit 115ee1cf3968d1027832852ed54304fffe60a77b. --- .../jetty/server/handler/ContextHandler.java | 4 +- .../java/org/eclipse/jetty/util/TypeUtil.java | 37 ++++++++++++++----- .../ee10/servlet/ServletContextHandler.java | 17 +++++---- .../jetty/ee10/servlet/SessionHandler.java | 5 ++- .../jetty/ee9/nested/ContextHandler.java | 13 ++++--- .../jetty/ee9/nested/SessionHandler.java | 5 ++- 6 files changed, 52 insertions(+), 29 deletions(-) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index 54dcf4235706..e09a9d9af9dd 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -653,11 +653,11 @@ protected void exitScope(Request request, Context lastContext, ClassLoader lastL */ protected void notifyExitScope(Request request) { - for (ContextScopeListener listener : TypeUtil.reverse(_contextListeners)) + for (ListIterator i = TypeUtil.listIteratorAtEnd(_contextListeners); i.hasPrevious();) { try { - listener.exitScope(_context, request); + i.previous().exitScope(_context, request); } catch (Throwable e) { diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java index d1658fdafd33..9dd18d63d389 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java @@ -34,6 +34,7 @@ import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.ListIterator; import java.util.Objects; import java.util.Optional; import java.util.ServiceConfigurationError; @@ -64,6 +65,7 @@ public class TypeUtil public static final int CR = '\r'; public static final int LF = '\n'; private static final Pattern TRAILING_DIGITS = Pattern.compile("^\\D*(\\d+)$"); + private static final ListIterator EMPTY_LIST_ITERATOR = Collections.EMPTY_LIST.listIterator(); private static final HashMap> name2Class = new HashMap<>(); @@ -174,18 +176,35 @@ public static List asList(T[] a) } /** - *

Returns a new list with the elements of the specified list in reverse order.

- *

The specified list is not modified, differently from {@link Collections#reverse(List)}.

- * - * @param list the list whose elements are to be reversed - * @return a new list with the elements in reverse order + *

Returns a {@link ListIterator} positioned at the last item in a list

+ *

The method is safe for concurrent modifications iff the list iterator itself is safe.

+ * @param list the list * @param the element type + * @return A {@link ListIterator} with {@link ListIterator#hasNext()} that would have return {@code false} + * at the snapshot of the list represented by this call. */ - public static List reverse(List list) + public static ListIterator listIteratorAtEnd(List list) { - List result = new ArrayList<>(list); - Collections.reverse(result); - return result; + try + { + int size = list.size(); + if (size == 0) + { + @SuppressWarnings("unchecked") + ListIterator emptyListIterator = (ListIterator)EMPTY_LIST_ITERATOR; + return emptyListIterator; + } + return list.listIterator(size); + } + catch (IndexOutOfBoundsException e) + { + // list was concurrently modified, so do this the hard way + ListIterator i = list.listIterator(); + while (i.hasNext()) + i.next(); + + return i; + } } /** diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java index eb6e4e081135..6de8dd52d56b 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java @@ -31,6 +31,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.ListIterator; import java.util.Locale; import java.util.Map; import java.util.Objects; @@ -519,11 +520,11 @@ public void contextDestroyed() throws Exception //Call context listeners Throwable multiException = null; ServletContextEvent event = new ServletContextEvent(getServletContext()); - for (ServletContextListener listener : TypeUtil.reverse(_destroyServletContextListeners)) + for (ListIterator i = TypeUtil.listIteratorAtEnd(_destroyServletContextListeners); i.hasPrevious();) { try { - callContextDestroyed(listener, event); + callContextDestroyed(i.previous(), event); } catch (Exception x) { @@ -574,17 +575,17 @@ protected void requestDestroyed(Request baseRequest, HttpServletRequest request) if (!_servletRequestListeners.isEmpty()) { final ServletRequestEvent sre = new ServletRequestEvent(getServletContext(), request); - for (ServletRequestListener listener : TypeUtil.reverse(_servletRequestListeners)) + for (ListIterator i = TypeUtil.listIteratorAtEnd(_servletRequestListeners); i.hasPrevious();) { - listener.requestDestroyed(sre); + i.previous().requestDestroyed(sre); } } if (!_servletRequestAttributeListeners.isEmpty()) { - for (ServletRequestAttributeListener listener : TypeUtil.reverse(_servletRequestAttributeListeners)) + for (ListIterator i = TypeUtil.listIteratorAtEnd(_servletRequestAttributeListeners); i.hasPrevious();) { - scopedRequest.removeEventListener(listener); + scopedRequest.removeEventListener(i.previous()); } } } @@ -1223,11 +1224,11 @@ protected void notifyExitScope(Request request) ServletContextRequest scopedRequest = Request.as(request, ServletContextRequest.class); if (!_contextListeners.isEmpty()) { - for (ServletContextScopeListener listener : TypeUtil.reverse(_contextListeners)) + for (ListIterator i = TypeUtil.listIteratorAtEnd(_contextListeners); i.hasPrevious(); ) { try { - listener.exitScope(getContext(), scopedRequest); + i.previous().exitScope(getContext(), scopedRequest); } catch (Throwable e) { diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/SessionHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/SessionHandler.java index 50bfa347b7fd..1b27c27b89e2 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/SessionHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/SessionHandler.java @@ -19,6 +19,7 @@ import java.util.EventListener; import java.util.Iterator; import java.util.List; +import java.util.ListIterator; import java.util.Locale; import java.util.Map; import java.util.Set; @@ -570,9 +571,9 @@ public void onSessionDestroyed(Session session) getSessionContext().run(() -> { HttpSessionEvent event = new HttpSessionEvent(session.getApi()); - for (HttpSessionListener listener : TypeUtil.reverse(_sessionListeners)) + for (ListIterator i = TypeUtil.listIteratorAtEnd(_sessionListeners); i.hasPrevious();) { - listener.sessionDestroyed(event); + i.previous().sessionDestroyed(event); } }); } diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java index 3e04fbf1277a..03d6e050c114 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java @@ -29,6 +29,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.ListIterator; import java.util.Locale; import java.util.Map; import java.util.Objects; @@ -1000,17 +1001,17 @@ protected void requestDestroyed(Request baseRequest, HttpServletRequest request) if (!_servletRequestListeners.isEmpty()) { final ServletRequestEvent sre = new ServletRequestEvent(_apiContext, request); - for (ServletRequestListener listener : TypeUtil.reverse(_servletRequestListeners)) + for (ListIterator i = TypeUtil.listIteratorAtEnd(_servletRequestListeners); i.hasPrevious();) { - listener.requestDestroyed(sre); + i.previous().requestDestroyed(sre); } } if (!_servletRequestAttributeListeners.isEmpty()) { - for (ServletRequestAttributeListener listener : TypeUtil.reverse(_servletRequestAttributeListeners)) + for (ListIterator i = TypeUtil.listIteratorAtEnd(_servletRequestAttributeListeners); i.hasPrevious();) { - baseRequest.removeEventListener(listener); + baseRequest.removeEventListener(i.previous()); } } } @@ -1070,11 +1071,11 @@ protected void exitScope(Request request) { if (!_contextListeners.isEmpty()) { - for (ContextScopeListener listener : TypeUtil.reverse(_contextListeners)) + for (ListIterator i = TypeUtil.listIteratorAtEnd(_contextListeners); i.hasPrevious();) { try { - listener.exitScope(_apiContext, request); + i.previous().exitScope(_apiContext, request); } catch (Throwable e) { diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SessionHandler.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SessionHandler.java index d49960cb0e61..977491280bff 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SessionHandler.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SessionHandler.java @@ -20,6 +20,7 @@ import java.util.Enumeration; import java.util.EventListener; import java.util.List; +import java.util.ListIterator; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import java.util.function.Function; @@ -835,9 +836,9 @@ public void onSessionDestroyed(Session session) Runnable r = () -> { HttpSessionEvent event = new HttpSessionEvent(session.getApi()); - for (HttpSessionListener listener : TypeUtil.reverse(_sessionListeners)) + for (ListIterator i = TypeUtil.listIteratorAtEnd(_sessionListeners); i.hasPrevious();) { - listener.sessionDestroyed(event); + i.previous().sessionDestroyed(event); } }; _contextHandler.getCoreContextHandler().getContext().run(r);