From e988d4804189eceea8095d85fcd99cb512991d26 Mon Sep 17 00:00:00 2001
From: Teppo Kurki <teppo.kurki@vaadin.com>
Date: Mon, 9 Dec 2024 08:42:25 +0200
Subject: [PATCH] fix: service destroy listener exceptions (#20622)

Related to #20577
---
 .../com/vaadin/flow/server/VaadinService.java | 17 ++++-
 .../com/vaadin/flow/server/VaadinServlet.java |  9 ++-
 .../vaadin/flow/server/VaadinServiceTest.java | 72 +++++++++++++++++++
 3 files changed, 93 insertions(+), 5 deletions(-)

diff --git a/flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java b/flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java
index b4e54ffccd4..9cda238fa81 100644
--- a/flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java
+++ b/flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java
@@ -2205,8 +2205,21 @@ public Registration addServiceDestroyListener(
      */
     public void destroy() {
         ServiceDestroyEvent event = new ServiceDestroyEvent(this);
-        serviceDestroyListeners
-                .forEach(listener -> listener.serviceDestroy(event));
+        RuntimeException exception = null;
+        for (ServiceDestroyListener listener : serviceDestroyListeners) {
+            try {
+                listener.serviceDestroy(event);
+            } catch (RuntimeException e) {
+                if (exception == null) {
+                    exception = e;
+                } else {
+                    e.addSuppressed(e);
+                }
+            }
+        }
+        if (exception != null) {
+            throw exception;
+        }
     }
 
     /**
diff --git a/flow-server/src/main/java/com/vaadin/flow/server/VaadinServlet.java b/flow-server/src/main/java/com/vaadin/flow/server/VaadinServlet.java
index f8804937e3b..0e075e8882e 100644
--- a/flow-server/src/main/java/com/vaadin/flow/server/VaadinServlet.java
+++ b/flow-server/src/main/java/com/vaadin/flow/server/VaadinServlet.java
@@ -623,10 +623,13 @@ static URL getApplicationUrl(HttpServletRequest request)
     @Override
     public void destroy() {
         super.destroy();
-        if (getService() != null) {
-            getService().destroy();
+        try {
+            if (getService() != null) {
+                getService().destroy();
+            }
+        } finally {
+            isServletInitialized = false;
         }
-        isServletInitialized = false;
     }
 
     private VaadinServletContext initializeContext() {
diff --git a/flow-server/src/test/java/com/vaadin/flow/server/VaadinServiceTest.java b/flow-server/src/test/java/com/vaadin/flow/server/VaadinServiceTest.java
index 97e54dc2028..5a319caea02 100644
--- a/flow-server/src/test/java/com/vaadin/flow/server/VaadinServiceTest.java
+++ b/flow-server/src/test/java/com/vaadin/flow/server/VaadinServiceTest.java
@@ -30,6 +30,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 
@@ -63,6 +64,7 @@
 
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertThrows;
 
 /**
  *
@@ -99,6 +101,34 @@ public void sessionDestroy(SessionDestroyEvent event) {
         }
     }
 
+    private class ThrowingSessionDestroyListener
+            implements SessionDestroyListener {
+
+        @Override
+        public void sessionDestroy(SessionDestroyEvent event) {
+            throw new RuntimeException();
+        }
+    }
+
+    private class TestServiceDestroyListener implements ServiceDestroyListener {
+
+        int callCount = 0;
+
+        @Override
+        public void serviceDestroy(ServiceDestroyEvent event) {
+            callCount++;
+        }
+    }
+
+    private class ThrowingServiceDestroyListener
+            implements ServiceDestroyListener {
+
+        @Override
+        public void serviceDestroy(ServiceDestroyEvent event) {
+            throw new RuntimeException();
+        }
+    }
+
     private String createCriticalNotification(String caption, String message,
             String details, String url) {
         return VaadinService.createCriticalNotificationJSON(caption, message,
@@ -298,6 +328,48 @@ public void testFireSessionDestroy()
                 1, listener.callCount);
     }
 
+    @Test
+    public void testSessionDestroyListenerCalled_whenAnotherListenerThrows()
+            throws ServiceException {
+        VaadinService service = createService();
+
+        ThrowingSessionDestroyListener throwingListener = new ThrowingSessionDestroyListener();
+        TestSessionDestroyListener listener = new TestSessionDestroyListener();
+
+        service.addSessionDestroyListener(throwingListener);
+        service.addSessionDestroyListener(listener);
+
+        final AtomicInteger errorCount = new AtomicInteger();
+        MockVaadinSession vaadinSession = new MockVaadinSession(service);
+        vaadinSession.lock();
+        vaadinSession.setErrorHandler(e -> errorCount.getAndIncrement());
+        vaadinSession.unlock();
+        service.fireSessionDestroy(vaadinSession);
+
+        Assert.assertEquals("ErrorHandler not called exactly once", 1,
+                errorCount.get());
+
+        Assert.assertEquals("SessionDestroyListener not called exactly once", 1,
+                listener.callCount);
+    }
+
+    @Test
+    public void testServiceDestroyListenerCalled_whenAnotherListenerThrows()
+            throws ServletException, ServiceException {
+        VaadinService service = createService();
+
+        ThrowingServiceDestroyListener throwingListener = new ThrowingServiceDestroyListener();
+        TestServiceDestroyListener listener = new TestServiceDestroyListener();
+
+        service.addServiceDestroyListener(throwingListener);
+        service.addServiceDestroyListener(listener);
+
+        assertThrows(RuntimeException.class, () -> service.destroy());
+
+        Assert.assertEquals("ServiceDestroyListener not called exactly once", 1,
+                listener.callCount);
+    }
+
     @Test
     public void captionIsSetToACriticalNotification() {
         String notification = createCriticalNotification("foobar", "message",