Issue #3361 thread safe addHandler

Updates from review:
 - undeploy destroys context
 - reinstated mapContexts

Signed-off-by: Greg Wilkins <gregw@webtide.com>

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

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

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_")
+    @ManagedOperation("Update the mapping of context path to context")
-    @Deprecated
     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;
+            }
+        });
     }
 
     /* ------------------------------------------------------------ */

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

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)