From 790dbbfaaa10809095da28c47537df1a1869b27a Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 21 Mar 2019 10:24:14 +1100 Subject: [PATCH] Issue #3361 thread safe addHandler Updates from review: - private fields - callbacks on deploy/undeploy - more deprecated javadoc Signed-off-by: Greg Wilkins --- .../deploy/bindings/StandardDeployer.java | 8 ++- .../deploy/bindings/StandardUndeployer.java | 14 ++--- .../handler/ContextHandlerCollection.java | 52 +++++++++++++++---- .../server/handler/HandlerCollection.java | 2 +- .../jetty/util/thread/SerializedExecutor.java | 24 ++++----- 5 files changed, 69 insertions(+), 31 deletions(-) diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/bindings/StandardDeployer.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/bindings/StandardDeployer.java index 7033f263214..df52116a8ec 100644 --- a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/bindings/StandardDeployer.java +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/bindings/StandardDeployer.java @@ -22,6 +22,7 @@ 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.util.SharedBlockingCallback; public class StandardDeployer implements AppLifeCycle.Binding { @@ -37,9 +38,12 @@ public class StandardDeployer implements AppLifeCycle.Binding { ContextHandler handler = app.getContextHandler(); if (handler == null) - { throw new NullPointerException("No Handler created for App: " + app); + + try(SharedBlockingCallback.Blocker blocker = new SharedBlockingCallback().acquire()) + { + app.getDeploymentManager().getContexts().deployHandler(handler, blocker); + blocker.block(); } - app.getDeploymentManager().getContexts().deployHandler(handler); } } 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 a04462f1e05..1996978a244 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 @@ -22,14 +22,10 @@ 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.log.Log; -import org.eclipse.jetty.util.log.Logger; +import org.eclipse.jetty.util.SharedBlockingCallback; public class StandardUndeployer implements AppLifeCycle.Binding { - private static final Logger LOG = Log.getLogger(StandardUndeployer.class); - @Override public String[] getBindingTargets() { @@ -41,7 +37,11 @@ public class StandardUndeployer implements AppLifeCycle.Binding public void processBinding(Node node, App app) throws Exception { ContextHandler handler = app.getContextHandler(); - ContextHandlerCollection chcoll = app.getDeploymentManager().getContexts(); - chcoll.undeployHandler(handler); + + try(SharedBlockingCallback.Blocker blocker = new SharedBlockingCallback().acquire()) + { + app.getDeploymentManager().getContexts().undeployHandler(handler, blocker); + blocker.block(); + } } } 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 cbdb52528d4..6eb3666fa59 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 @@ -35,6 +35,7 @@ import org.eclipse.jetty.server.HttpChannelState; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.util.ArrayTernaryTrie; import org.eclipse.jetty.util.ArrayUtil; +import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.Trie; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.annotation.ManagedOperation; @@ -58,6 +59,7 @@ public class ContextHandlerCollection extends HandlerCollection private static final Logger LOG = Log.getLogger(ContextHandlerCollection.class); private final SerializedExecutor _serializedExecutor = new SerializedExecutor(); + @Deprecated private Class _contextClass = ContextHandler.class; /* ------------------------------------------------------------ */ @@ -74,7 +76,10 @@ public class ContextHandlerCollection extends HandlerCollection } /* ------------------------------------------------------------ */ - @ManagedOperation("update the mapping of context path to context") + /** + * @deprecated Mapping is now always maintained on every set/add + */ + @ManagedOperation("update the mapping of context path to context (no longer required_") @Deprecated public void mapContexts() { @@ -227,6 +232,7 @@ public class ContextHandlerCollection extends HandlerCollection * @param contextPath The context path to add * @param resourceBase the base (root) Resource * @return the ContextHandler just added + * @deprecated Unused convenience method no longer supported. */ @Deprecated public ContextHandler addContext(String contextPath,String resourceBase) @@ -252,17 +258,29 @@ public class ContextHandlerCollection extends HandlerCollection *

* This method is the equivalent of {@link #addHandler(Handler)}, * but its execution is non-block and mutually excluded from all - * other calls to {@link #deployHandler(Handler)} and - * {@link #undeployHandler(Handler)}. + * other calls to {@link #deployHandler(Handler, Callback)} and + * {@link #undeployHandler(Handler, Callback)}. * The handler may be added after this call returns. *

* @param handler the handler to deploy + * @param callback Called after handler has been added */ - public void deployHandler(Handler handler) + public void deployHandler(Handler handler, Callback callback) { if (handler.getServer()!=getServer()) handler.setServer(getServer()); - _serializedExecutor.execute(()-> addHandler(handler)); + _serializedExecutor.execute(()-> + { + try + { + addHandler(handler); + callback.succeeded(); + } + catch(Throwable th) + { + callback.failed(th); + } + }); } /* ------------------------------------------------------------ */ @@ -271,21 +289,35 @@ public class ContextHandlerCollection extends HandlerCollection *

* This method is the equivalent of {@link #removeHandler(Handler)}, * but its execution is non-block and mutually excluded from all - * other calls to {@link #deployHandler(Handler)} and - * {@link #undeployHandler(Handler)}. + * other calls to {@link #deployHandler(Handler,Callback)} and + * {@link #undeployHandler(Handler,Callback)}. * The handler may be removed after this call returns. *

* @param handler The handler to undeploy + * @param callback Called after handler has been removed */ - public void undeployHandler(Handler handler) + public void undeployHandler(Handler handler, Callback callback) { - _serializedExecutor.execute(()-> removeHandler(handler)); + _serializedExecutor.execute(()-> + { + try + { + removeHandler(handler); + callback.succeeded(); + } + catch(Throwable th) + { + callback.failed(th); + } + }); } /* ------------------------------------------------------------ */ /** * @return The class to use to add new Contexts + * @deprecated Unused convenience mechanism not used. */ + @Deprecated public Class getContextClass() { return _contextClass; @@ -294,7 +326,9 @@ public class ContextHandlerCollection extends HandlerCollection /* ------------------------------------------------------------ */ /** * @param contextClass The class to use to add new Contexts + * @deprecated Unused convenience mechanism not used. */ + @Deprecated public void setContextClass(Class contextClass) { if (contextClass ==null || !(ContextHandler.class.isAssignableFrom(contextClass))) 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 c90be145f21..7e97e73d80e 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 @@ -246,7 +246,7 @@ public class HandlerCollection extends AbstractHandlerContainer /* ------------------------------------------------------------ */ protected static class Handlers { - final Handler[] _handlers; + private final Handler[] _handlers; protected Handlers(Handler[] handlers) { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/SerializedExecutor.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/SerializedExecutor.java index cb141187a0a..fb398d6bccd 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/SerializedExecutor.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/SerializedExecutor.java @@ -69,26 +69,26 @@ public class SerializedExecutor implements Executor } finally { - // Are we the current last Link? - if (_last.compareAndSet(link, null)) - return; - - // not the last task, so its next link will eventually be set - Link next = link._next.get(); - while (next == null) + // Are we are not the current last Link? + if (!_last.compareAndSet(link, null)) { - Thread.yield(); // Thread.onSpinWait(); - next = link._next.get(); + // continue running the list, but may need to wait for the next link + Link next = link._next.get(); + while (next == null) + { + Thread.yield(); // Thread.onSpinWait(); + next = link._next.get(); + } + link = next; } - link = next; } } } private class Link { - final Runnable _task; - final AtomicReference _next = new AtomicReference<>(); + private final Runnable _task; + private final AtomicReference _next = new AtomicReference<>(); public Link(Runnable task) {