Fix racy HTTP tests by waiting for requests to finish

Ensure the background Jetty threads have been able to write the
request log record before the JUnit thread tries to read the set
of requests back.  This wait is necessary because the JUnit thread
may be able to continue as soon as Jetty has finished writing
the response onto the socket, and hasn't necessarily finished the
post-response logging activity.

By using a semaphore with a fixed number of resources, and using
one resource per request, but all of them when we want to read the
log, we implement a simple lock that requires there be no active
requests when we want to get the log from the JUnit thread.

Change-Id: I499e1c96418557185d0e19ba8befe892f26ce7e4
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/util/AppServer.java

@@ -64,7 +64,6 @@ import org.eclipse.jetty.server.Connector;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.UserIdentity;
 import org.eclipse.jetty.server.handler.ContextHandlerCollection;
-import org.eclipse.jetty.server.handler.RequestLogHandler;
 import org.eclipse.jetty.server.nio.SelectChannelConnector;
 import org.eclipse.jetty.servlet.ServletContextHandler;
 import org.eclipse.jetty.util.thread.QueuedThreadPool;
@@ -126,15 +125,12 @@ public class AppServer {
 		contexts = new ContextHandlerCollection();
 
 		log = new TestRequestLog();
-
+		log.setHandler(contexts);
-		final RequestLogHandler logHandler = new RequestLogHandler();
-		logHandler.setHandler(contexts);
-		logHandler.setRequestLog(log);
 
 		server = new Server();
 		server.setConnectors(new Connector[] { connector });
 		server.setThreadPool(pool);
-		server.setHandler(logHandler);
+		server.setHandler(log);
 
 		server.setStopAtShutdown(false);
 		server.setGracefulShutdown(0);

org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/util/TestRequestLog.java

@@ -43,29 +43,94 @@
 
 package org.eclipse.jgit.http.test.util;
 
+import org.eclipse.jetty.server.DispatcherType;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Response;
+import org.eclipse.jetty.server.handler.HandlerWrapper;
+
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.Semaphore;
 
-import org.eclipse.jetty.server.Request;
+import javax.servlet.ServletException;
-import org.eclipse.jetty.server.RequestLog;
+import javax.servlet.http.HttpServletRequest;
-import org.eclipse.jetty.server.Response;
+import javax.servlet.http.HttpServletResponse;
-import org.eclipse.jetty.util.component.AbstractLifeCycle;
 
 /** Logs request made through {@link AppServer}. */
-class TestRequestLog extends AbstractLifeCycle implements RequestLog {
+class TestRequestLog extends HandlerWrapper {
-	private final List<AccessEvent> events = new ArrayList<AccessEvent>();
+  private static final int MAX = 16;
-
+
-	/** Reset the log back to its original empty state. */
+  private final List<AccessEvent> events = new ArrayList<AccessEvent>();
-	synchronized void clear() {
+
-		events.clear();
+  private final Semaphore active = new Semaphore(MAX);
-	}
+
-
+  /** Reset the log back to its original empty state. */
-	/** @return all of the events made since the last clear. */
+  void clear() {
-	synchronized List<AccessEvent> getEvents() {
+    try {
-		return events;
+      for (;;) {
-	}
+        try {
-
+          active.acquire(MAX);
-	public synchronized void log(Request request, Response response) {
+          break;
-		events.add(new AccessEvent(request, response));
+        } catch (InterruptedException e) {
-	}
+          continue;
+        }
+      }
+
+      synchronized (events) {
+        events.clear();
+      }
+    } finally {
+      active.release(MAX);
+    }
+  }
+
+  /** @return all of the events made since the last clear. */
+  List<AccessEvent> getEvents() {
+    try {
+      for (;;) {
+        try {
+          active.acquire(MAX);
+          break;
+        } catch (InterruptedException e) {
+          continue;
+        }
+      }
+
+      synchronized (events) {
+        return events;
+      }
+    } finally {
+      active.release(MAX);
+    }
+  }
+
+  @Override
+  public void handle(String target, Request baseRequest, HttpServletRequest request,
+      HttpServletResponse response) throws IOException, ServletException {
+    try {
+      for (;;) {
+        try {
+          active.acquire();
+          break;
+        } catch (InterruptedException e) {
+          continue;
+        }
+      }
+
+      super.handle(target, baseRequest, request, response);
+
+      if (DispatcherType.REQUEST.equals(baseRequest.getDispatcherType()))
+        log((Request) request, (Response) response);
+
+    } finally {
+      active.release();
+    }
+  }
+
+  private void log(Request request, Response response) {
+    synchronized (events) {
+      events.add(new AccessEvent(request, response));
+    }
+  }
 }