Issue #3361 thread safe addHandler

Updates from review:
 - private fields
 - callbacks on deploy/undeploy
 - more deprecated javadoc

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2019-03-21 10:24:14 +11:00
parent 2f2c9f2f3f
commit 790dbbfaaa
5 changed files with 69 additions and 31 deletions

View File

@ -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);
}
}

View File

@ -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();
}
}
}

View File

@ -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<? extends ContextHandler> _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
* <p>
* 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.
* </p>
* @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
* <p>
* 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.
* </p>
* @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<? extends ContextHandler> contextClass)
{
if (contextClass ==null || !(ContextHandler.class.isAssignableFrom(contextClass)))

View File

@ -246,7 +246,7 @@ public class HandlerCollection extends AbstractHandlerContainer
/* ------------------------------------------------------------ */
protected static class Handlers
{
final Handler[] _handlers;
private final Handler[] _handlers;
protected Handlers(Handler[] handlers)
{

View File

@ -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<Link> _next = new AtomicReference<>();
private final Runnable _task;
private final AtomicReference<Link> _next = new AtomicReference<>();
public Link(Runnable task)
{