From 1c956dc470f5173985c0cab18f3ce8753d261987 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 26 Mar 2019 14:15:36 +1100 Subject: [PATCH] Issue #3361 thread safe addHandler Updates from review: - undeploy destroys context - reinstated mapContexts Signed-off-by: Greg Wilkins --- .../deploy/bindings/StandardUndeployer.java | 9 +++++--- .../jetty/server/handler/ContextHandler.java | 5 ++-- .../handler/ContextHandlerCollection.java | 22 ++++++++++++++---- .../server/handler/HandlerCollection.java | 3 ++- .../handler/ContextHandlerCollectionTest.java | 23 +++++++++---------- 5 files changed, 40 insertions(+), 22 deletions(-) diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/bindings/StandardUndeployer.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/bindings/StandardUndeployer.java index 5381aa0a4d3..4b36a5e2834 100644 --- a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/bindings/StandardUndeployer.java +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/bindings/StandardUndeployer.java @@ -18,10 +18,12 @@ package org.eclipse.jetty.deploy.bindings; + import org.eclipse.jetty.deploy.App; import org.eclipse.jetty.deploy.AppLifeCycle; import org.eclipse.jetty.deploy.graph.Node; import org.eclipse.jetty.server.handler.ContextHandler; +import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.util.Callback; public class StandardUndeployer implements AppLifeCycle.Binding @@ -36,10 +38,11 @@ public class StandardUndeployer implements AppLifeCycle.Binding @Override public void processBinding(Node node, App app) throws Exception { - ContextHandler handler = app.getContextHandler(); - + ContextHandlerCollection contexts = app.getDeploymentManager().getContexts(); + ContextHandler context = app.getContextHandler(); Callback.Completable blocker = new Callback.Completable(); - app.getDeploymentManager().getContexts().undeployHandler(handler, blocker); + contexts.undeployHandler(context, blocker); blocker.get(); + context.destroy(); } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index b2fd1f42cb4..6b8fbcc8be5 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -1605,9 +1605,10 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu if (getServer() != null && (getServer().isStarting() || getServer().isStarted())) { - Handler[] contextCollections = getServer().getChildHandlersByClass(ContextHandlerCollection.class); + ContextHandlerCollection[] contextCollections = + (ContextHandlerCollection[])getServer().getChildHandlersByClass(ContextHandlerCollection.class); for (int h = 0; contextCollections != null && h < contextCollections.length; h++) - ((ContextHandlerCollection)contextCollections[h]).mapContexts(); + contextCollections[h].mapContexts(); } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandlerCollection.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandlerCollection.java index c4c7119910a..fd3fbae21af 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandlerCollection.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandlerCollection.java @@ -77,13 +77,27 @@ public class ContextHandlerCollection extends HandlerCollection /* ------------------------------------------------------------ */ /** - * @deprecated Mapping is now always maintained on every set/add + * Remap the contexts. Normally this is not required as context + * mapping is maintained as a side effect of {@link #setHandlers(Handler[])} + * However, if configuration changes in the deep handler structure (eg contextpath is changed), then + * this call will trigger a remapping. + * This method is mutually excluded from {@link #deployHandler(Handler, Callback)} and + * {@link #undeployHandler(Handler, Callback)} */ - @ManagedOperation("update the mapping of context path to context (no longer required_") - @Deprecated + @ManagedOperation("Update the mapping of context path to context") public void mapContexts() { - LOG.warn("mapContexts is Deprecated"); + _serializedExecutor.execute(()-> + { + while(true) + { + Handlers handlers = _handlers.get(); + if (handlers==null) + break; + if (updateHandlers(handlers, newHandlers(handlers.getHandlers()))) + break; + } + }); } /* ------------------------------------------------------------ */ diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/HandlerCollection.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/HandlerCollection.java index e00fe5eb52e..e39077489ab 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/HandlerCollection.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/HandlerCollection.java @@ -176,13 +176,14 @@ public class HandlerCollection extends AbstractHandlerContainer /** * Adds a handler. * This implementation adds the passed handler to the end of the existing collection of handlers. + * If the handler is already added, it is removed and readded */ public void addHandler(Handler handler) { while(true) { Handlers old = _handlers.get(); - Handlers handlers = newHandlers(ArrayUtil.addToArray(old==null?null:old._handlers, handler, Handler.class)); + Handlers handlers = newHandlers(ArrayUtil.addToArray(old==null?null:ArrayUtil.removeFromArray(old._handlers, handler), handler, Handler.class)); if (updateHandlers(old,handlers)) break; } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerCollectionTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerCollectionTest.java index b0eda81f5c3..d6b04538afe 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerCollectionTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerCollectionTest.java @@ -18,16 +18,6 @@ package org.eclipse.jetty.server.handler; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.endsWith; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.startsWith; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; - import java.io.IOException; import javax.servlet.AsyncContext; @@ -41,9 +31,18 @@ import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.hamcrest.Matchers; - import org.junit.jupiter.api.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + public class ContextHandlerCollectionTest { @Test @@ -214,7 +213,7 @@ public class ContextHandlerCollectionTest IsHandledHandler handler = (IsHandledHandler)context.getHandler(); context.setVirtualHosts(contextHosts); - // trigger this manually; it's supposed to be called when adding the handler + // trigger this manually handlerCollection.mapContexts(); for(String host : requestHosts)