Skip to content

Commit

Permalink
8296919: Make system tests that detect memory leaks more robust by us…
Browse files Browse the repository at this point in the history 
…ing JMemoryBuddy utility

Reviewed-by: angorya, kcr
  • Loading branch information
Lukasz Kostyra authored and Andy Goryachev committed May 8, 2023
1 parent 27ee7e3 commit d9a82d1
Show file tree
Hide file tree
Showing 11 changed files with 94 additions and 170 deletions.
3 changes: 2 additions & 1 deletion build.gradle
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -3488,6 +3488,7 @@ project(":web") {
implementation project(":graphics")
implementation project(":controls")
implementation project(":media")
testImplementation project(":base").sourceSets.test.output
}

compileJava.dependsOn updateCacheIfNeeded
Expand Down
33 changes: 33 additions & 0 deletions modules/javafx.base/src/test/java/test/util/memory/JMemoryBuddy.java
View file
Expand Up @@ -32,6 +32,7 @@
import java.lang.management.ManagementFactory;
import java.lang.ref.WeakReference;
import java.text.SimpleDateFormat;
import java.util.Collection;
import java.util.Date;
import java.util.LinkedList;
import java.util.Objects;
Expand Down Expand Up @@ -91,6 +92,38 @@ public static void assertCollectable(WeakReference weakReference) {
}
}

/**
* Checks whether the content of all references from a WeakReference array can be collected.
* @param weakReferences WeakReference objects to check.
*/
public static void assertCollectable(WeakReference[] weakReferences) {
for (WeakReference wr : weakReferences) {
assertCollectable(wr);
}
}

/**
* Checks whether the content of all references from a WeakReference Collection can be collected.
* @param weakReferences WeakReference objects to check.
*/
public static <T> void assertCollectable(Collection<WeakReference<T>> weakReferences) {
for (WeakReference<T> wr : weakReferences) {
assertCollectable(wr);
}
}

/**
* Checks whether the content of provided WeakReference objects can be collected.
* @param weakReference The WeakReference to check.
* @param otherWeakReferences Other WeakReference objects to check.
*/
public static void assertCollectable(WeakReference weakReference, WeakReference... otherWeakReferences) {
assertCollectable(weakReference);
for (WeakReference wr : otherWeakReferences) {
assertCollectable(wr);
}
}

/**
* Checks whether the content of the WeakReference can be collected.
* @param weakReference The WeakReference to check.
Expand Down
2 changes: 1 addition & 1 deletion modules/javafx.web/.classpath
View file
Expand Up @@ -42,7 +42,7 @@
<classpathentry combineaccessrules="false" kind="src" path="/base">
<attributes>
<attribute name="module" value="true"/>
<attribute name="add-exports" value="javafx.base/com.sun.javafx=javafx.web"/>
<attribute name="add-exports" value="javafx.base/com.sun.javafx=javafx.web:javafx.base/test.util.memory=javafx.web"/>
</attributes>
</classpathentry>
<classpathentry kind="con" path="org.eclipse.jdt.junit.JUNIT_CONTAINER/5">
Expand Down
55 changes: 8 additions & 47 deletions modules/javafx.web/src/test/java/test/javafx/scene/web/LeakTest.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -47,6 +47,7 @@
import org.w3c.dom.Element;
import org.w3c.dom.NodeList;
import static org.junit.Assert.*;
import test.util.memory.JMemoryBuddy;

public class LeakTest extends TestBase {

Expand Down Expand Up @@ -79,7 +80,7 @@ public class LeakTest extends TestBase {

@Test public void testGarbageCollectability() throws InterruptedException {
final int count = 3;
Reference<?>[] willGC = new Reference[count];
WeakReference<?>[] willGC = new WeakReference[count];

submit(() -> {
WebView webView = new WebView();
Expand All @@ -88,21 +89,7 @@ public class LeakTest extends TestBase {
willGC[2] = new WeakReference<>(WebEngineShim.getPage(webView.getEngine()));
});

Thread.sleep(SLEEP_TIME);

for (int i = 0; i < 5; i++) {
System.gc();

if (isAllElementsNull(willGC)) {
break;
}

Thread.sleep(SLEEP_TIME);
}

assertNull("WebView has not been GCed", willGC[0].get());
assertNull("WebEngine has not been GCed", willGC[1].get());
assertNull("WebPage has not been GCed", willGC[2].get());
JMemoryBuddy.assertCollectable(willGC);
}

private static boolean isAllElementsNull(Reference<?>[] array) {
Expand All @@ -116,7 +103,7 @@ private static boolean isAllElementsNull(Reference<?>[] array) {

@Test public void testJSObjectGarbageCollectability() throws InterruptedException {
final int count = 10000;
Reference<?>[] willGC = new Reference[count];
WeakReference<?>[] willGC = new WeakReference[count];

submit(() -> {
for (int i = 0; i < count; i++) {
Expand All @@ -125,19 +112,7 @@ private static boolean isAllElementsNull(Reference<?>[] array) {
}
});

Thread.sleep(SLEEP_TIME);

for (int i = 0; i < 5; i++) {
System.gc();

if (isAllElementsNull(willGC)) {
break;
}

Thread.sleep(SLEEP_TIME);
}

assertTrue("All JSObjects are GC'ed", isAllElementsNull(willGC));
JMemoryBuddy.assertCollectable(willGC);
}

// JDK-8170938
Expand Down Expand Up @@ -177,7 +152,7 @@ private State getLoadState() {
// JDK-8176729
@Test public void testDOMNodeDisposeCount() throws InterruptedException {
int count = 7;
Reference<?>[] willGC = new Reference[count];
WeakReference<?>[] willGC = new WeakReference[count];
final String html =
"<html>\n" +
"<head></head>\n" +
Expand Down Expand Up @@ -236,20 +211,6 @@ private State getLoadState() {
assertEquals("Expected NodeImpl(tag:br) HashCount", initialHashCount+7, NodeImplShim.test_getHashCount());
});

Thread.sleep(SLEEP_TIME);

for (int i = 0; i < 5; i++) {
System.gc();

if (isAllElementsNull(willGC)) {
break;
}

Thread.sleep(SLEEP_TIME);
}

// Give disposer a chance to run
Thread.sleep(SLEEP_TIME);
assertEquals("NodeImpl HashCount after dispose", initialHashCount, NodeImplShim.test_getHashCount());
JMemoryBuddy.assertCollectable(willGC);
}
}
32 changes: 12 additions & 20 deletions tests/system/src/test/java/test/javafx/embed/swing/SwingNodeDnDMemoryLeakTest.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -33,6 +33,9 @@
import java.util.concurrent.CountDownLatch;

import javax.swing.JLabel;
import javax.swing.SwingUtilities;

import java.lang.reflect.InvocationTargetException;

import javafx.application.Application;
import javafx.embed.swing.SwingNode;
Expand All @@ -45,12 +48,12 @@
import org.junit.Test;

import test.util.Util;
import test.util.memory.JMemoryBuddy;

public class SwingNodeDnDMemoryLeakTest {

final static int TOTAL_SWINGNODE = 10;
static CountDownLatch launchLatch = new CountDownLatch(1);
final static int GC_ATTEMPTS = 10;
ArrayList<WeakReference<SwingNode>> weakRefArrSN =
new ArrayList(TOTAL_SWINGNODE);

Expand All @@ -65,12 +68,16 @@ public static void teardownOnce() {
}

@Test
public void testSwingNodeMemoryLeak() {
public void testSwingNodeMemoryLeak() throws InvocationTargetException, InterruptedException {
Util.runAndWait(() -> {
testSwingNodeObjectsInStage();
});
attemptGCSwingNode();
assertEquals(TOTAL_SWINGNODE, getCleanedUpSwingNodeCount());

// Invoke a noop on EDT thread and wait for a bit to make sure EDT processed node objects
SwingUtilities.invokeAndWait(() -> {});
Util.sleep(500);

JMemoryBuddy.assertCollectable(weakRefArrSN);
}

private void testSwingNodeObjectsInStage() {
Expand Down Expand Up @@ -112,21 +119,6 @@ private void testSwingNodeObjectsInStage() {
}
}

private void attemptGCSwingNode() {
// Attempt gc GC_ATTEMPTS times
for (int i = 0; i < GC_ATTEMPTS; i++) {
System.gc();
if (getCleanedUpSwingNodeCount() == TOTAL_SWINGNODE) {
break;
}
try {
Thread.sleep(500);
} catch (InterruptedException e) {
System.err.println("InterruptedException occurred during Thread.sleep()");
}
}
}

private int getCleanedUpSwingNodeCount() {
int count = 0;
for (WeakReference<SwingNode> ref : weakRefArrSN) {
Expand Down
49 changes: 11 additions & 38 deletions tests/system/src/test/java/test/javafx/embed/swing/SwingNodeMemoryLeakTest.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -29,10 +29,12 @@
import static org.junit.Assume.assumeTrue;

import java.lang.ref.WeakReference;
import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
import java.util.concurrent.CountDownLatch;

import javax.swing.JLabel;
import javax.swing.SwingUtilities;

import javafx.application.Application;
import javafx.embed.swing.SwingNode;
Expand All @@ -47,6 +49,7 @@
import com.sun.javafx.PlatformUtil;

import test.util.Util;
import test.util.memory.JMemoryBuddy;

public class SwingNodeMemoryLeakTest {

Expand All @@ -69,18 +72,17 @@ public static void teardownOnce() {
}

@Test
public void testSwingNodeMemoryLeak() {
if (PlatformUtil.isMac()) {
assumeTrue(Boolean.getBoolean("unstable.test")); // JDK-8196614
}
public void testSwingNodeMemoryLeak() throws InterruptedException, InvocationTargetException {
Util.runAndWait(() -> {
testSwingNodeObjectsInStage();
});
attemptGCSwingNode();
assertEquals(TOTAL_SWINGNODE, getCleanedUpSwingNodeCount());

attemptGCJLabel();
assertEquals(TOTAL_SWINGNODE, getCleanedUpJLabelCount());
// Invoke a noop on EDT thread and wait for a bit to make sure EDT processed node objects
SwingUtilities.invokeAndWait(() -> {});
Util.sleep(500);

JMemoryBuddy.assertCollectable(weakRefArrSN);
JMemoryBuddy.assertCollectable(weakRefArrJL);
}

private void testSwingNodeObjectsInStage() {
Expand Down Expand Up @@ -120,35 +122,6 @@ private void testSwingNodeObjectsInStage() {
}
}

private void attemptGCSwingNode() {
// Attempt gc GC_ATTEMPTS times
for (int i = 0; i < GC_ATTEMPTS; i++) {
System.gc();
if (getCleanedUpSwingNodeCount() == TOTAL_SWINGNODE) {
break;
}
try {
Thread.sleep(250);
} catch (InterruptedException e) {
System.err.println("InterruptedException occurred during Thread.sleep()");
}
}
}

private void attemptGCJLabel() {
// Attempt gc GC_ATTEMPTS times
for (int i = 0; i < GC_ATTEMPTS; i++) {
System.gc();
if (getCleanedUpJLabelCount() == TOTAL_SWINGNODE) {
break;
}
try {
Thread.sleep(250);
} catch (InterruptedException e) {
System.err.println("InterruptedException occurred during Thread.sleep()");
}
}
}
private int getCleanedUpSwingNodeCount() {
int count = 0;
for (WeakReference<SwingNode> ref : weakRefArrSN) {
Expand Down
28 changes: 13 additions & 15 deletions tests/system/src/test/java/test/javafx/scene/control/AccordionTitlePaneLeakTest.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -42,6 +42,7 @@
import org.junit.Test;

import test.util.Util;
import test.util.memory.JMemoryBuddy;

public class AccordionTitlePaneLeakTest {

Expand All @@ -50,6 +51,8 @@ public class AccordionTitlePaneLeakTest {
static private StackPane root;
static private Stage stage;

private WeakReference<TitledPane> weakRefToPane;

public static class TestApp extends Application {
@Override
public void start(Stage primaryStage) throws Exception {
Expand Down Expand Up @@ -77,19 +80,14 @@ public static void teardownOnce() {

@Test
public void testForTitledPaneLeak() throws Exception {
TitledPane pane = new TitledPane();
accordion.getPanes().add(pane);
WeakReference<TitledPane> weakRefToPane = new WeakReference<>(pane);
pane = null;
accordion.getPanes().clear();
for (int i = 0; i < 10; i++) {
System.gc();
if (weakRefToPane.get() == null) {
break;
}
Util.sleep(500);
}
// Ensure accordion's skin no longer hold a ref to titled pane.
Assert.assertNull("Couldn't collect TitledPane", weakRefToPane.get());
Util.runAndWait(() -> {
TitledPane pane = new TitledPane();
accordion.getPanes().add(pane);
weakRefToPane = new WeakReference<>(pane);
pane = null;
accordion.getPanes().clear();
});

JMemoryBuddy.assertCollectable(weakRefToPane);
}
}

1 comment on commit d9a82d1

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.