Skip to content

Commit

Permalink
#10226 - assert using awaitility and fix heap dump cleanup when a lea…
Browse files Browse the repository at this point in the history 
…k is detected

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
  • Loading branch information
@lorban @olamy
lorban authored and olamy committed Aug 24, 2023
1 parent f2832a8 commit 7246620
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 21 deletions.
7 changes: 7 additions & 0 deletions jetty-core/jetty-fcgi/jetty-fcgi-server/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
<argLine>
@{argLine} ${jetty.surefire.argLine}
--add-reads org.eclipse.jetty.fcgi.server=org.eclipse.jetty.logging
--add-reads org.eclipse.jetty.fcgi.server=java.management
--add-reads org.eclipse.jetty.fcgi.server=jdk.management
</argLine>
</configuration>
</plugin>
Expand Down Expand Up @@ -61,5 +63,10 @@
<artifactId>jetty-unixdomain-server</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.awaitility</groupId>
<artifactId>awaitility</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
</project>
63 changes: 59 additions & 4 deletions ...fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/AbstractHttpClientServerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@

package org.eclipse.jetty.fcgi.server;

import java.io.IOException;
import java.lang.management.ManagementFactory;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Comparator;
import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;
import javax.management.MBeanServer;

import com.sun.management.HotSpotDiagnosticMXBean;
import org.awaitility.Awaitility;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.HttpClientTransport;
import org.eclipse.jetty.fcgi.client.transport.HttpClientTransportOverFCGI;
Expand All @@ -28,8 +39,9 @@
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.TestInfo;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.fail;

public abstract class AbstractHttpClientServerTest
{
Expand Down Expand Up @@ -67,19 +79,62 @@ public void start(Handler handler) throws Exception
}

@AfterEach
public void dispose()
public void dispose(TestInfo testInfo) throws Exception
{
try
{
if (serverBufferPool != null)
assertThat("Server Leaks: " + serverBufferPool.getLeaks(), serverBufferPool.getLeaks().size(), Matchers.is(0));
{
try
{
Awaitility.await().atMost(3, TimeUnit.SECONDS).until(() -> serverBufferPool.getLeaks().size(), Matchers.is(0));
}
catch (Exception e)
{
String className = testInfo.getTestClass().orElseThrow().getName();
dumpHeap("server-" + className);
fail(e.getMessage() + "\n---\nServer Leaks: " + serverBufferPool.dumpLeaks() + "---\n");
}
}
if (clientBufferPool != null)
assertThat("Client Leaks: " + clientBufferPool.getLeaks(), clientBufferPool.getLeaks().size(), Matchers.is(0));
{
try
{
Awaitility.await().atMost(3, TimeUnit.SECONDS).until(() -> clientBufferPool.getLeaks().size(), Matchers.is(0));
}
catch (Exception e)
{
String className = testInfo.getTestClass().orElseThrow().getName();
dumpHeap("client-" + className);
fail(e.getMessage() + "\n---\nClient Leaks: " + clientBufferPool.dumpLeaks() + "---\n");
}
}
}
finally
{
LifeCycle.stop(client);
LifeCycle.stop(server);
}
}

private static void dumpHeap(String testMethodName) throws IOException
{
Path targetDir = Path.of("target/leaks");
if (Files.exists(targetDir))
{
try (Stream<Path> stream = Files.walk(targetDir))
{
stream.sorted(Comparator.reverseOrder())
.map(Path::toFile)
.forEach(java.io.File::delete);
}
}
Files.createDirectories(targetDir);
String dumpName = targetDir.resolve(testMethodName + ".hprof").toString();

MBeanServer server = ManagementFactory.getPlatformMBeanServer();
HotSpotDiagnosticMXBean mxBean = ManagementFactory.newPlatformMXBeanProxy(
server, "com.sun.management:type=HotSpotDiagnostic", HotSpotDiagnosticMXBean.class);
mxBean.dumpHeap(dumpName, true);
}
}
42 changes: 29 additions & 13 deletions ...ty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/HttpClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ public boolean handle(org.eclipse.jetty.server.Request request, org.eclipse.jett
String paramValue2 = fields.getValue(paramName2);
assertEquals("", paramValue2);
Content.Sink.write(response, true, UTF_8.encode("empty"));
callback.succeeded();
return true;
}
});
Expand Down Expand Up @@ -274,6 +275,7 @@ public boolean handle(org.eclipse.jetty.server.Request request, org.eclipse.jett
}
String paramValue2 = fields.getValue(paramName2);
Content.Sink.write(response, true, UTF_8.encode(paramValue2));
callback.succeeded();
return true;
}
});
Expand Down Expand Up @@ -650,33 +652,46 @@ public boolean handle(org.eclipse.jetty.server.Request request, org.eclipse.jett
@Test
public void testLongPollIsAbortedWhenClientIsStopped() throws Exception
{
AtomicReference<Callback> callbackRef = new AtomicReference<>();
CountDownLatch latch = new CountDownLatch(1);
start(new Handler.Abstract()
{
@Override
public boolean handle(org.eclipse.jetty.server.Request request, org.eclipse.jetty.server.Response response, Callback callback)
{
latch.countDown();
// Do not complete the callback.
// Do not complete the callback, but store it aside for
// releasing the buffer later on.
callbackRef.set(callback);
return true;
}
});

CountDownLatch completeLatch = new CountDownLatch(1);
client.newRequest("localhost", connector.getLocalPort())
.scheme(scheme)
.send(result ->
{
if (result.isFailed())
completeLatch.countDown();
});
try
{
CountDownLatch completeLatch = new CountDownLatch(1);
client.newRequest("localhost", connector.getLocalPort())
.scheme(scheme)
.send(result ->
{
if (result.isFailed())
completeLatch.countDown();
});

assertTrue(latch.await(5, TimeUnit.SECONDS));
assertTrue(latch.await(5, TimeUnit.SECONDS));

// Stop the client, the complete listener must be invoked.
client.stop();
// Stop the client, the complete listener must be invoked.
client.stop();

assertTrue(completeLatch.await(5, TimeUnit.SECONDS));
assertTrue(completeLatch.await(5, TimeUnit.SECONDS));
}
finally
{
// Release the buffer.
Callback callback = callbackRef.get();
if (callback != null)
callback.succeeded();
}
}

@Test
Expand Down Expand Up @@ -757,6 +772,7 @@ public boolean handle(org.eclipse.jetty.server.Request request, org.eclipse.jett
{
Content.Sink.write(response, false, UTF_8.encode("A"));
Content.Sink.write(response, true, UTF_8.encode("B"));
callback.succeeded();
return true;
}
});
Expand Down
11 changes: 7 additions & 4 deletions ...client-transports/src/test/java/org/eclipse/jetty/test/client/transport/AbstractTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,14 @@ public void dispose(TestInfo testInfo) throws Exception
private static void dumpHeap(String testMethodName) throws IOException
{
Path targetDir = Path.of("target/leaks");
try (Stream<Path> stream = Files.walk(targetDir))
if (Files.exists(targetDir))
{
stream.sorted(Comparator.reverseOrder())
.map(Path::toFile)
.forEach(java.io.File::delete);
try (Stream<Path> stream = Files.walk(targetDir))
{
stream.sorted(Comparator.reverseOrder())
.map(Path::toFile)
.forEach(java.io.File::delete);
}
}
Files.createDirectories(targetDir);
String dumpName = targetDir.resolve(testMethodName + ".hprof").toString();
Expand Down

0 comments on commit 7246620

Please sign in to comment.