Issue #4662 Ensure contextDestroyed called after filters and servlets destroyed (#4667)

* Issue #4662 Ensure contextDestroyed called after filters and servlets destroyed

Signed-off-by: Jan Bartel <janb@webtide.com>
This commit is contained in:
Jan Bartel 2020-04-06 10:12:19 +02:00 committed by GitHub
parent f79b5aa507
commit cf0e3c530f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 303 additions and 58 deletions

View File

@ -182,6 +182,14 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu
__serverInfo = serverInfo;
}
public enum ContextStatus
{
NOTSET,
INITIALIZED,
DESTROYED
}
protected ContextStatus _contextStatus = ContextStatus.NOTSET;
protected Context _scontext;
private final AttributesMap _attributes;
private final Map<String, String> _initParams;
@ -828,6 +836,8 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu
// defers the calling of super.doStart()
startContext();
contextInitialized();
_availability = Availability.AVAILABLE;
LOG.info("Started {}", this);
@ -886,49 +896,97 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu
addEventListener(new ManagedAttributeListener(this, StringUtil.csvSplit(managedAttributes)));
super.doStart();
}
/**
* Call the ServletContextListeners contextInitialized methods.
* This can be called from a ServletHandler during the proper sequence
* of initializing filters, servlets and listeners. However, if there is
* no ServletHandler, the ContextHandler will call this method during
* doStart().
*
* @throws Exception
*/
public void contextInitialized() throws Exception
{
// Call context listeners
_destroyServletContextListeners.clear();
if (!_servletContextListeners.isEmpty())
switch (_contextStatus)
{
ServletContextEvent event = new ServletContextEvent(_scontext);
for (ServletContextListener listener : _servletContextListeners)
case NOTSET:
{
callContextInitialized(listener, event);
_destroyServletContextListeners.add(listener);
try
{
_destroyServletContextListeners.clear();
if (!_servletContextListeners.isEmpty())
{
ServletContextEvent event = new ServletContextEvent(_scontext);
for (ServletContextListener listener : _servletContextListeners)
{
callContextInitialized(listener, event);
_destroyServletContextListeners.add(listener);
}
}
}
finally
{
_contextStatus = ContextStatus.INITIALIZED;
}
break;
}
default:
break;
}
}
/**
* Call the ServletContextListeners with contextDestroyed.
* This method can be called from a ServletHandler in the
* proper sequence of destroying filters, servlets and listeners.
* If there is no ServletHandler, the ContextHandler must ensure
* these listeners are called instead.
*
* @throws Exception
*/
public void contextDestroyed() throws Exception
{
switch (_contextStatus)
{
case INITIALIZED:
{
try
{
//Call context listeners
MultiException ex = new MultiException();
ServletContextEvent event = new ServletContextEvent(_scontext);
Collections.reverse(_destroyServletContextListeners);
for (ServletContextListener listener : _destroyServletContextListeners)
{
try
{
callContextDestroyed(listener, event);
}
catch (Exception x)
{
ex.add(x);
}
}
ex.ifExceptionThrow();
}
finally
{
_contextStatus = ContextStatus.DESTROYED;
}
break;
}
default:
break;
}
}
protected void stopContext() throws Exception
{
// Call the context listeners
ServletContextEvent event = new ServletContextEvent(_scontext);
Collections.reverse(_destroyServletContextListeners);
MultiException ex = new MultiException();
for (ServletContextListener listener : _destroyServletContextListeners)
{
try
{
callContextDestroyed(listener, event);
}
catch (Exception x)
{
ex.add(x);
}
}
// stop all the handler hierarchy
try
{
super.doStop();
}
catch (Exception x)
{
ex.add(x);
}
ex.ifExceptionThrow();
super.doStop();
}
protected void callContextInitialized(ServletContextListener l, ServletContextEvent e)
@ -945,9 +1003,6 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu
l.contextDestroyed(e);
}
/*
* @see org.eclipse.thread.AbstractLifeCycle#doStop()
*/
@Override
protected void doStop() throws Exception
{
@ -987,6 +1042,8 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu
stopContext();
contextDestroyed();
// retain only durable listeners
setEventListeners(_durableListeners.toArray(new EventListener[0]));
_durableListeners.clear();
@ -1019,6 +1076,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu
}
finally
{
_contextStatus = ContextStatus.NOTSET;
__context.set(oldContext);
exitScope(null);
LOG.info("Stopped {}", this);

View File

@ -27,6 +27,9 @@ import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import javax.servlet.ServletContextEvent;
import javax.servlet.ServletContextListener;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@ -445,6 +448,22 @@ public class ContextHandlerTest
assertThat(connector.getResponse("GET /foo/xxx HTTP/1.0\n\n"), Matchers.containsString("ctx='/foo'"));
assertThat(connector.getResponse("GET /foo/bar/xxx HTTP/1.0\n\n"), Matchers.containsString("ctx='/foo/bar'"));
}
@Test
public void testContextInitializationDestruction() throws Exception
{
Server server = new Server();
ContextHandlerCollection contexts = new ContextHandlerCollection();
server.setHandler(contexts);
ContextHandler noServlets = new ContextHandler(contexts, "/noservlets");
TestServletContextListener listener = new TestServletContextListener();
noServlets.addEventListener(listener);
server.start();
assertEquals(1, listener.initialized);
server.stop();
assertEquals(1, listener.destroyed);
}
@Test
public void testContextVirtualGetContext() throws Exception
@ -840,4 +859,22 @@ public class ContextHandlerTest
writer.println("ctx='" + request.getContextPath() + "'");
}
}
private static class TestServletContextListener implements ServletContextListener
{
public int initialized = 0;
public int destroyed = 0;
@Override
public void contextInitialized(ServletContextEvent sce)
{
initialized++;
}
@Override
public void contextDestroyed(ServletContextEvent sce)
{
destroyed++;
}
}
}

View File

@ -33,6 +33,7 @@ import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.ConcurrentMap;
import java.util.function.Consumer;
import java.util.stream.Stream;
import javax.servlet.DispatcherType;
import javax.servlet.Filter;
@ -307,6 +308,9 @@ public class ServletHandler extends ScopedHandler
_servlets = shs;
ServletMapping[] sms = (ServletMapping[])LazyList.toArray(servletMappings, ServletMapping.class);
_servletMappings = sms;
if (_contextHandler != null)
_contextHandler.contextDestroyed();
//Retain only Listeners added via jetty apis (is Source.EMBEDDED)
List<ListenerHolder> listenerHolders = new ArrayList<>();
@ -733,30 +737,40 @@ public class ServletHandler extends ScopedHandler
public void initialize()
throws Exception
{
_initialized = true;
MultiException mx = new MultiException();
Stream.concat(Stream.concat(
Arrays.stream(_filters),
Arrays.stream(_servlets).sorted()),
Arrays.stream(_listeners))
.forEach(h ->
Consumer<BaseHolder<?>> c = h ->
{
try
{
try
if (!h.isStarted())
{
if (!h.isStarted())
{
h.start();
h.initialize();
}
h.start();
h.initialize();
}
catch (Throwable e)
{
LOG.debug(Log.EXCEPTION, e);
mx.add(e);
}
});
}
catch (Throwable e)
{
LOG.debug(Log.EXCEPTION, e);
mx.add(e);
}
};
//Start the listeners so we can call them
Arrays.stream(_listeners).forEach(c);
//call listeners contextInitialized
if (_contextHandler != null)
_contextHandler.contextInitialized();
//Only set initialized true AFTER the listeners have been called
_initialized = true;
//Start the filters then the servlets
Stream.concat(
Arrays.stream(_filters),
Arrays.stream(_servlets).sorted())
.forEach(c);
mx.ifExceptionThrow();
}

View File

@ -35,6 +35,7 @@ import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
import javax.servlet.FilterRegistration;
import javax.servlet.GenericServlet;
import javax.servlet.Servlet;
import javax.servlet.ServletContainerInitializer;
import javax.servlet.ServletContext;
@ -106,6 +107,75 @@ public class ServletContextHandlerTest
private LocalConnector _connector;
private static final AtomicInteger __testServlets = new AtomicInteger();
private static int __initIndex = 0;
private static int __destroyIndex = 0;
public class StopTestFilter implements Filter
{
int _initIndex;
int _destroyIndex;
@Override
public void init(FilterConfig filterConfig) throws ServletException
{
_initIndex = __initIndex++;
}
@Override
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException,
ServletException
{
}
@Override
public void destroy()
{
_destroyIndex = __destroyIndex++;
}
}
public class StopTestServlet extends GenericServlet
{
int _initIndex;
int _destroyIndex;
@Override
public void destroy()
{
_destroyIndex = __destroyIndex++;
super.destroy();
}
@Override
public void init() throws ServletException
{
_initIndex = __initIndex++;
super.init();
}
@Override
public void service(ServletRequest req, ServletResponse res) throws ServletException, IOException
{
}
}
public class StopTestListener implements ServletContextListener
{
int _initIndex;
int _destroyIndex;
@Override
public void contextInitialized(ServletContextEvent sce)
{
_initIndex = __initIndex++;
}
@Override
public void contextDestroyed(ServletContextEvent sce)
{
_destroyIndex = __destroyIndex++;
}
}
public static class MySCI implements ServletContainerInitializer
{
@ -405,6 +475,37 @@ public class ServletContextHandlerTest
_server.join();
}
@Test
public void testDestroyOrder() throws Exception
{
ContextHandlerCollection contexts = new ContextHandlerCollection();
_server.setHandler(contexts);
ServletContextHandler root = new ServletContextHandler(contexts, "/", ServletContextHandler.SESSIONS);
ListenerHolder listenerHolder = new ListenerHolder();
StopTestListener stopTestListener = new StopTestListener();
listenerHolder.setListener(stopTestListener);
root.getServletHandler().addListener(listenerHolder);
ServletHolder servletHolder = new ServletHolder();
StopTestServlet stopTestServlet = new StopTestServlet();
servletHolder.setServlet(stopTestServlet);
root.addServlet(servletHolder, "/test");
FilterHolder filterHolder = new FilterHolder();
StopTestFilter stopTestFilter = new StopTestFilter();
filterHolder.setFilter(stopTestFilter);
root.addFilter(filterHolder, "/*", EnumSet.of(DispatcherType.REQUEST));
_server.start();
_server.stop();
assertEquals(0, stopTestListener._initIndex); //listeners contextInitialized called first
assertEquals(1, stopTestFilter._initIndex); //filters init
assertEquals(2, stopTestServlet._initIndex); //servlets init
assertEquals(0, stopTestFilter._destroyIndex); //filters destroyed first
assertEquals(1, stopTestServlet._destroyIndex); //servlets destroyed next
assertEquals(2, stopTestListener._destroyIndex); //listener contextDestroyed last
}
@Test
public void testAddSessionListener() throws Exception
{
@ -436,6 +537,41 @@ public class ServletContextHandlerTest
assertTrue((Boolean)root.getServletContext().getAttribute("MyContextListener.contextInitialized"));
}
@Test
public void testContextInitializationDestruction() throws Exception
{
Server server = new Server();
ContextHandlerCollection contexts = new ContextHandlerCollection();
server.setHandler(contexts);
ServletContextHandler root = new ServletContextHandler(contexts, "/");
class TestServletContextListener implements ServletContextListener
{
public int initialized = 0;
public int destroyed = 0;
@Override
public void contextInitialized(ServletContextEvent sce)
{
initialized++;
}
@Override
public void contextDestroyed(ServletContextEvent sce)
{
destroyed++;
}
}
TestServletContextListener listener = new TestServletContextListener();
root.addEventListener(listener);
server.start();
server.stop();
assertEquals(1, listener.initialized);
server.stop();
assertEquals(1, listener.destroyed);
}
@Test
public void testListenersFromContextListener() throws Exception
{

View File

@ -60,8 +60,8 @@ public class ServletLifeCycleTest
context.getObjectFactory().addDecorator(new TestDecorator());
ServletHandler sh = context.getServletHandler();
sh.addListener(new ListenerHolder(TestListener.class));
context.addEventListener(context.getServletContext().createListener(TestListener2.class));
sh.addListener(new ListenerHolder(TestListener.class)); //added directly to ServletHandler
context.addEventListener(context.getServletContext().createListener(TestListener2.class));//create,decorate and add listener to context - no holder!
sh.addFilterWithMapping(TestFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST));
sh.addFilterWithMapping(new FilterHolder(context.getServletContext().createFilter(TestFilter2.class)), "/*", EnumSet.of(DispatcherType.REQUEST));
@ -110,8 +110,6 @@ public class ServletLifeCycleTest
server.stop();
assertThat(events, Matchers.contains(
"contextDestroyed class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestListener",
"contextDestroyed class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestListener2",
"destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestFilter2",
"Destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestFilter2",
"destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestFilter",
@ -122,6 +120,8 @@ public class ServletLifeCycleTest
"destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestServlet2",
"Destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestServlet",
"destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestServlet",
"contextDestroyed class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestListener",
"contextDestroyed class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestListener2",
"Destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestListener"
));