Merge pull request #6415 from eclipse/jetty-10.0.x-6383-FixFileBufferedResponseHandlerTest

Issue #6383 - Fix flaky test FileBufferedResponseHandlerTest

jetty-server/src/main/java/org/eclipse/jetty/server/handler/BufferedResponseHandler.java

@@ -203,7 +203,7 @@ public class BufferedResponseHandler extends HandlerWrapper
     {
     }
 
-    private class ArrayBufferedInterceptor implements BufferedInterceptor
+    class ArrayBufferedInterceptor implements BufferedInterceptor
     {
         private final Interceptor _next;
         private final HttpChannel _channel;

jetty-server/src/main/java/org/eclipse/jetty/server/handler/FileBufferedResponseHandler.java

@@ -71,7 +71,7 @@ public class FileBufferedResponseHandler extends BufferedResponseHandler
         return new FileBufferedInterceptor(httpChannel, interceptor);
     }
 
-    private class FileBufferedInterceptor implements BufferedResponseHandler.BufferedInterceptor
+    class FileBufferedInterceptor implements BufferedResponseHandler.BufferedInterceptor
     {
         private static final int MAX_MAPPED_BUFFER_SIZE = Integer.MAX_VALUE / 2;
 
@@ -100,7 +100,7 @@ public class FileBufferedResponseHandler extends BufferedResponseHandler
             BufferedInterceptor.super.resetBuffer();
         }
 
-        private void dispose()
+        protected void dispose()
         {
             IO.close(_fileOutputStream);
             _fileOutputStream = null;

jetty-server/src/test/java/org/eclipse/jetty/server/handler/FileBufferedResponseHandlerTest.java

@@ -11,7 +11,7 @@
 // ========================================================================
 //
 
-package org.eclipse.jetty.server;
+package org.eclipse.jetty.server.handler;
 
 import java.io.File;
 import java.io.IOException;
@@ -25,6 +25,7 @@ import java.nio.file.Path;
 import java.time.Duration;
 import java.util.Random;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
 import javax.servlet.ServletException;
@@ -34,9 +35,14 @@ import javax.servlet.http.HttpServletResponse;
 
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.HttpTester;
-import org.eclipse.jetty.server.handler.AbstractHandler;
-import org.eclipse.jetty.server.handler.FileBufferedResponseHandler;
-import org.eclipse.jetty.server.handler.HandlerCollection;
+import org.eclipse.jetty.server.HttpChannel;
+import org.eclipse.jetty.server.HttpConfiguration;
+import org.eclipse.jetty.server.HttpConnectionFactory;
+import org.eclipse.jetty.server.HttpOutput;
+import org.eclipse.jetty.server.LocalConnector;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.toolchain.test.FS;
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
 import org.eclipse.jetty.util.Callback;
@@ -58,6 +64,7 @@ public class FileBufferedResponseHandlerTest
 {
     private static final Logger LOG = LoggerFactory.getLogger(FileBufferedResponseHandlerTest.class);
 
+    private final CountDownLatch _disposeLatch = new CountDownLatch(1);
     private Server _server;
     private LocalConnector _localConnector;
     private ServerConnector _serverConnector;
@@ -81,7 +88,22 @@ public class FileBufferedResponseHandlerTest
         _serverConnector = new ServerConnector(_server, new HttpConnectionFactory(config));
         _server.addConnector(_serverConnector);
 
-        _bufferedHandler = new FileBufferedResponseHandler();
+        _bufferedHandler = new FileBufferedResponseHandler()
+        {
+            @Override
+            protected BufferedInterceptor newBufferedInterceptor(HttpChannel httpChannel, HttpOutput.Interceptor interceptor)
+            {
+                return new FileBufferedInterceptor(httpChannel, interceptor)
+                {
+                    @Override
+                    protected void dispose()
+                    {
+                        super.dispose();
+                        _disposeLatch.countDown();
+                    }
+                };
+            }
+        };
         _bufferedHandler.setTempDir(_testDir);
         _bufferedHandler.getPathIncludeExclude().include("/include/*");
         _bufferedHandler.getPathIncludeExclude().exclude("*.exclude");
@@ -152,6 +174,8 @@ public class FileBufferedResponseHandlerTest
         assertThat(response.getStatus(), is(HttpStatus.OK_200));
         assertThat(responseContent, containsString("Committed: false"));
         assertThat(responseContent, containsString("NumFiles: 1"));
+
+        assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
         assertThat(getNumFiles(), is(0));
     }
 
@@ -244,6 +268,8 @@ public class FileBufferedResponseHandlerTest
         assertThat(responseContent, containsString("NumFilesBeforeFlush: 0"));
         assertThat(responseContent, containsString("Committed: false"));
         assertThat(responseContent, containsString("NumFiles: 1"));
+
+        assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
         assertThat(getNumFiles(), is(0));
     }
 
@@ -274,6 +300,8 @@ public class FileBufferedResponseHandlerTest
         assertThat(response.getStatus(), is(HttpStatus.OK_200));
         assertThat(responseContent, not(containsString("writtenAfterClose")));
         assertThat(responseContent, containsString("NumFiles: 1"));
+
+        assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
         assertThat(getNumFiles(), is(0));
     }
 
@@ -334,6 +362,8 @@ public class FileBufferedResponseHandlerTest
         // The flush should not create the file unless there is content to write.
         assertThat(response.getStatus(), is(HttpStatus.OK_200));
         assertThat(responseContent, containsString("NumFiles: 0"));
+
+        assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
         assertThat(getNumFiles(), is(0));
     }
 
@@ -373,6 +403,8 @@ public class FileBufferedResponseHandlerTest
         assertThat(responseContent, containsString("NumFilesBeforeReset: 1"));
         assertThat(responseContent, containsString("NumFilesAfterReset: 0"));
         assertThat(responseContent, containsString("NumFilesAfterWrite: 1"));
+
+        assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
         assertThat(getNumFiles(), is(0));
     }
 
@@ -446,6 +478,8 @@ public class FileBufferedResponseHandlerTest
         assertThat(response.get("NumFiles"), is("1"));
         assertThat(response.get("FileSize"), is(Long.toString(fileSize)));
         assertThat(received.get(), is(fileSize));
+
+        assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
         assertThat(getNumFiles(), is(0));
     }
 
@@ -520,6 +554,7 @@ public class FileBufferedResponseHandlerTest
         assertThat(error.getMessage(), containsString("intentionally throwing from interceptor"));
 
         // All files were deleted.
+        assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
         assertThat(getNumFiles(), is(0));
     }
 
@@ -568,6 +603,7 @@ public class FileBufferedResponseHandlerTest
         assertThat(error, instanceOf(NoSuchFileException.class));
 
         // No files were created.
+        assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
         assertThat(getNumFiles(), is(0));
     }