From d032beddb5ea99efda34deb64fd8fb0f1f9a9f99 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 12 Jun 2019 14:11:13 +0200 Subject: [PATCH] Thread-safe removal of destruction callbacks in web scopes Closes gh-23117 --- .../web/context/ContextCleanupListener.java | 20 +++++++++++-------- .../request/ServletRequestAttributes.java | 7 +++---- .../context/support/ServletContextScope.java | 18 +++++++++++------ 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/context/ContextCleanupListener.java b/spring-web/src/main/java/org/springframework/web/context/ContextCleanupListener.java index 4742be00efb..5635fe57529 100644 --- a/spring-web/src/main/java/org/springframework/web/context/ContextCleanupListener.java +++ b/spring-web/src/main/java/org/springframework/web/context/ContextCleanupListener.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -54,22 +54,26 @@ public class ContextCleanupListener implements ServletContextListener { /** - * Find all ServletContext attributes which implement {@link DisposableBean} - * and destroy them, removing all affected ServletContext attributes eventually. - * @param sc the ServletContext to check + * Find all Spring-internal ServletContext attributes which implement + * {@link DisposableBean} and invoke the destroy method on them. + * @param servletContext the ServletContext to check + * @see DisposableBean#destroy() */ - static void cleanupAttributes(ServletContext sc) { - Enumeration attrNames = sc.getAttributeNames(); + static void cleanupAttributes(ServletContext servletContext) { + Enumeration attrNames = servletContext.getAttributeNames(); while (attrNames.hasMoreElements()) { String attrName = attrNames.nextElement(); if (attrName.startsWith("org.springframework.")) { - Object attrValue = sc.getAttribute(attrName); + Object attrValue = servletContext.getAttribute(attrName); if (attrValue instanceof DisposableBean) { try { ((DisposableBean) attrValue).destroy(); } catch (Throwable ex) { - logger.error("Couldn't invoke destroy method of attribute with name '" + attrName + "'", ex); + if (logger.isWarnEnabled()) { + logger.warn("Invocation of destroy method failed on ServletContext " + + "attribute with name '" + attrName + "'", ex); + } } } } diff --git a/spring-web/src/main/java/org/springframework/web/context/request/ServletRequestAttributes.java b/spring-web/src/main/java/org/springframework/web/context/request/ServletRequestAttributes.java index 281dbb36141..53e39ee716f 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/ServletRequestAttributes.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/ServletRequestAttributes.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -188,8 +188,8 @@ public class ServletRequestAttributes extends AbstractRequestAttributes { public void removeAttribute(String name, int scope) { if (scope == SCOPE_REQUEST) { if (isRequestActive()) { - this.request.removeAttribute(name); removeRequestDestructionCallback(name); + this.request.removeAttribute(name); } } else { @@ -197,9 +197,8 @@ public class ServletRequestAttributes extends AbstractRequestAttributes { if (session != null) { this.sessionAttributesToUpdate.remove(name); try { - session.removeAttribute(name); - // Remove any registered destruction callback as well. session.removeAttribute(DESTRUCTION_CALLBACK_NAME_PREFIX + name); + session.removeAttribute(name); } catch (IllegalStateException ex) { // Session invalidated - shouldn't usually happen. diff --git a/spring-web/src/main/java/org/springframework/web/context/support/ServletContextScope.java b/spring-web/src/main/java/org/springframework/web/context/support/ServletContextScope.java index 03d348566c4..eb850ef194b 100644 --- a/spring-web/src/main/java/org/springframework/web/context/support/ServletContextScope.java +++ b/spring-web/src/main/java/org/springframework/web/context/support/ServletContextScope.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -78,8 +78,10 @@ public class ServletContextScope implements Scope, DisposableBean { public Object remove(String name) { Object scopedObject = this.servletContext.getAttribute(name); if (scopedObject != null) { + synchronized (this.destructionCallbacks) { + this.destructionCallbacks.remove(name); + } this.servletContext.removeAttribute(name); - this.destructionCallbacks.remove(name); return scopedObject; } else { @@ -89,7 +91,9 @@ public class ServletContextScope implements Scope, DisposableBean { @Override public void registerDestructionCallback(String name, Runnable callback) { - this.destructionCallbacks.put(name, callback); + synchronized (this.destructionCallbacks) { + this.destructionCallbacks.put(name, callback); + } } @Override @@ -112,10 +116,12 @@ public class ServletContextScope implements Scope, DisposableBean { */ @Override public void destroy() { - for (Runnable runnable : this.destructionCallbacks.values()) { - runnable.run(); + synchronized (this.destructionCallbacks) { + for (Runnable runnable : this.destructionCallbacks.values()) { + runnable.run(); + } + this.destructionCallbacks.clear(); } - this.destructionCallbacks.clear(); } }