From 61938da3c22d62dc80c993a321ac2c6096510e08 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Leif=20=C3=85strand?= Date: Tue, 7 Feb 2017 10:07:07 +0200 Subject: [PATCH] Use thread-safe collections for VaadinService listeners (#7037) Fixes #7250 --- .../java/com/vaadin/server/VaadinService.java | 97 +++++++++++-------- 1 file changed, 57 insertions(+), 40 deletions(-) diff --git a/server/src/main/java/com/vaadin/server/VaadinService.java b/server/src/main/java/com/vaadin/server/VaadinService.java index 3050e41f0e..6d0c442785 100644 --- a/server/src/main/java/com/vaadin/server/VaadinService.java +++ b/server/src/main/java/com/vaadin/server/VaadinService.java @@ -25,7 +25,6 @@ import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.io.Serializable; import java.lang.reflect.Constructor; -import java.lang.reflect.Method; import java.net.MalformedURLException; import java.net.URL; import java.util.ArrayList; @@ -37,6 +36,8 @@ import java.util.Locale; import java.util.Map; import java.util.ServiceLoader; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Lock; @@ -51,7 +52,6 @@ import javax.servlet.ServletContext; import javax.servlet.http.HttpServletResponse; import com.vaadin.annotations.PreserveOnRefresh; -import com.vaadin.event.EventRouter; import com.vaadin.server.VaadinSession.FutureAccess; import com.vaadin.server.VaadinSession.State; import com.vaadin.server.communication.AtmospherePushConnection; @@ -66,7 +66,6 @@ import com.vaadin.shared.Registration; import com.vaadin.shared.ui.ui.UIConstants; import com.vaadin.ui.UI; import com.vaadin.util.CurrentInstance; -import com.vaadin.util.ReflectTools; import elemental.json.Json; import elemental.json.JsonException; @@ -101,17 +100,6 @@ public abstract class VaadinService implements Serializable { @Deprecated static final String REINITIALIZING_SESSION_MARKER = PRESERVE_UNBOUND_SESSION_ATTRIBUTE; - private static final Method SESSION_INIT_METHOD = ReflectTools.findMethod( - SessionInitListener.class, "sessionInit", SessionInitEvent.class); - - private static final Method SESSION_DESTROY_METHOD = ReflectTools - .findMethod(SessionDestroyListener.class, "sessionDestroy", - SessionDestroyEvent.class); - - private static final Method SERVICE_DESTROY_METHOD = ReflectTools - .findMethod(ServiceDestroyListener.class, "serviceDestroy", - ServiceDestroyEvent.class); - /** * @deprecated As of 7.0. Only supported for {@link LegacyApplication}. */ @@ -128,7 +116,18 @@ public abstract class VaadinService implements Serializable { private final DeploymentConfiguration deploymentConfiguration; - private final EventRouter eventRouter = new EventRouter(); + /* + * Can't use EventRouter for these listeners since it's not thread safe. One + * option would be to use an EventRouter instance guarded with a lock, but + * then we would needlessly hold a "global" lock while invoking potentially + * slow listener implementations. + */ + private final Set serviceDestroyListeners = Collections + .newSetFromMap(new ConcurrentHashMap<>()); + + private final List sessionInitListeners = new CopyOnWriteArrayList<>(); + + private final List sessionDestroyListeners = new CopyOnWriteArrayList<>(); private SystemMessagesProvider systemMessagesProvider = DefaultSystemMessagesProvider .get(); @@ -445,8 +444,8 @@ public abstract class VaadinService implements Serializable { * @return a registration object for removing the listener */ public Registration addSessionInitListener(SessionInitListener listener) { - return eventRouter.addListener(SessionInitEvent.class, listener, - SESSION_INIT_METHOD); + sessionInitListeners.add(listener); + return () -> sessionInitListeners.remove(listener); } /** @@ -463,8 +462,7 @@ public abstract class VaadinService implements Serializable { */ @Deprecated public void removeSessionInitListener(SessionInitListener listener) { - eventRouter.removeListener(SessionInitEvent.class, listener, - SESSION_INIT_METHOD); + sessionInitListeners.remove(listener); } /** @@ -482,8 +480,8 @@ public abstract class VaadinService implements Serializable { */ public Registration addSessionDestroyListener( SessionDestroyListener listener) { - return eventRouter.addListener(SessionDestroyEvent.class, listener, - SESSION_DESTROY_METHOD); + sessionDestroyListeners.add(listener); + return () -> sessionDestroyListeners.remove(listener); } /** @@ -517,12 +515,20 @@ public abstract class VaadinService implements Serializable { session.removeUI(ui); }); } - // for now, use the session error handler; in the future, could - // have an API for using some other handler for session init and - // destroy listeners - eventRouter.fireEvent( - new SessionDestroyEvent(VaadinService.this, session), - session.getErrorHandler()); + SessionDestroyEvent event = new SessionDestroyEvent( + VaadinService.this, session); + for (SessionDestroyListener listener : sessionDestroyListeners) { + try { + listener.sessionDestroy(event); + } catch (Exception e) { + /* + * for now, use the session error handler; in the future, + * could have an API for using some other handler for + * session init and destroy listeners + */ + session.getErrorHandler().error(new ErrorEvent(e)); + } + } session.setState(State.CLOSED); }); } @@ -540,8 +546,7 @@ public abstract class VaadinService implements Serializable { */ @Deprecated public void removeSessionDestroyListener(SessionDestroyListener listener) { - eventRouter.removeListener(SessionDestroyEvent.class, listener, - SESSION_DESTROY_METHOD); + sessionDestroyListeners.remove(listener); } /** @@ -851,12 +856,19 @@ public abstract class VaadinService implements Serializable { private void onVaadinSessionStarted(VaadinRequest request, VaadinSession session) throws ServiceException { - // for now, use the session error handler; in the future, could have an - // API for using some other handler for session init and destroy - // listeners - - eventRouter.fireEvent(new SessionInitEvent(this, session, request), - session.getErrorHandler()); + SessionInitEvent event = new SessionInitEvent(this, session, request); + for (SessionInitListener listener : sessionInitListeners) { + try { + listener.sessionInit(event); + } catch (Exception e) { + /* + * for now, use the session error handler; in the future, could + * have an API for using some other handler for session init and + * destroy listeners + */ + session.getErrorHandler().error(new ErrorEvent(e)); + } + } ServletPortletHelper.checkUiProviders(session, this); } @@ -1888,6 +1900,10 @@ public abstract class VaadinService implements Serializable { /** * Adds a service destroy listener that gets notified when this service is * destroyed. + *

+ * The listeners may be invoked in a non-deterministic order. In particular, + * it is not guaranteed that listeners will be invoked in the order they + * were added. * * @since 7.2 * @param listener @@ -1900,8 +1916,8 @@ public abstract class VaadinService implements Serializable { */ public Registration addServiceDestroyListener( ServiceDestroyListener listener) { - return eventRouter.addListener(ServiceDestroyEvent.class, listener, - SERVICE_DESTROY_METHOD); + serviceDestroyListeners.add(listener); + return () -> serviceDestroyListeners.remove(listener); } /** @@ -1917,8 +1933,7 @@ public abstract class VaadinService implements Serializable { */ @Deprecated public void removeServiceDestroyListener(ServiceDestroyListener listener) { - eventRouter.removeListener(ServiceDestroyEvent.class, listener, - SERVICE_DESTROY_METHOD); + serviceDestroyListeners.remove(serviceDestroyListeners); } /** @@ -1933,7 +1948,9 @@ public abstract class VaadinService implements Serializable { * @since 7.2 */ public void destroy() { - eventRouter.fireEvent(new ServiceDestroyEvent(this)); + ServiceDestroyEvent event = new ServiceDestroyEvent(this); + serviceDestroyListeners + .forEach(listener -> listener.serviceDestroy(event)); } /** -- 2.39.5