Bug 569752 - Detect non disposed OS resources via Object.finalize()
New "org.eclipse.swt.graphics.Resource.reportNonDisposed" system
property is added that can be used to track not disposed resources:
- if set to "true", SWT will report not disposed SWT resource without
allocating exception at SWT resource creation time.
- if set to "stacks", SWT will aditionally report the stack recorded at
resource creation time.
A custom NonDisposedReporter can be added to customize how not disposed
SWT resources should be reported.
Note: Fonts created from native handle are ignored because they don't
own the handle and can't be disposed. Same for Region on macOS.
Change-Id: I3bfdc9d5b41eba2cac5f187ea1fe771f9ff36750
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
diff --git a/bundles/org.eclipse.swt.browser.chromium/META-INF/MANIFEST.MF b/bundles/org.eclipse.swt.browser.chromium/META-INF/MANIFEST.MF
index 8567c89..655c1ff 100644
--- a/bundles/org.eclipse.swt.browser.chromium/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.swt.browser.chromium/META-INF/MANIFEST.MF
@@ -3,7 +3,7 @@
Bundle-Name: Chromium SWT Widget
Bundle-Vendor: Eclipse.org
Bundle-SymbolicName: org.eclipse.swt.browser.chromium;singleton:=true
-Bundle-Version: 3.115.200.qualifier
+Bundle-Version: 3.116.0.qualifier
Bundle-ManifestVersion: 2
Eclipse-PlatformFilter: (osgi.arch=x86_64)
SWT-Arch: x86_64
diff --git a/bundles/org.eclipse.swt.browser.chromium/pom.xml b/bundles/org.eclipse.swt.browser.chromium/pom.xml
index ce3f6af..eef7ee1 100644
--- a/bundles/org.eclipse.swt.browser.chromium/pom.xml
+++ b/bundles/org.eclipse.swt.browser.chromium/pom.xml
@@ -21,7 +21,7 @@
</parent>
<groupId>org.eclipse.swt</groupId>
<artifactId>org.eclipse.swt.browser.chromium</artifactId>
- <version>3.115.200-SNAPSHOT</version>
+ <version>3.116.0-SNAPSHOT</version>
<packaging>eclipse-plugin</packaging>
<properties>
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/Font.java b/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/Font.java
index a62cf05..a3c20ed 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/Font.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/Font.java
@@ -292,6 +292,12 @@
public static Font cocoa_new(Device device, NSFont handle) {
Font font = new Font(device);
font.handle = handle;
+ /*
+ * When created this way, Font doesn't own its .handle, and
+ * for this reason it can't be disposed. Tell leak detector
+ * to just ignore it.
+ */
+ font.nonDisposedIgnore = true;
return font;
}
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/Region.java b/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/Region.java
index 304368e..adfbcbe 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/Region.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/Region.java
@@ -97,6 +97,12 @@
Region(Device device, long handle) {
super(device);
this.handle = handle;
+ /*
+ * When created this way, Font doesn't own its .handle, and
+ * for this reason it can't be disposed. Tell leak detector
+ * to just ignore it.
+ */
+ this.nonDisposedIgnore = true;
}
/**
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/Resource.java b/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/Resource.java
index 625b416..ffce5e0 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/Resource.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/Resource.java
@@ -41,10 +41,71 @@
public abstract class Resource {
/**
+ * Stack recorded at resource allocation time, wraped as Exception for
+ * convenience
+ *
+ * @since 3.116
+ */
+ public static class NonDisposedException extends Exception {
+ static final long serialVersionUID = 1;
+ }
+
+ /**
+ * Reports not disposed SWT resource
+ *
+ * @since 3.116
+ */
+ public interface NonDisposedReporter {
+ /**
+ * Note that this is called as part of {@link Object#finalize()}, please
+ * read its docs.
+ *
+ * @param resource the Resource which wasn't properly disposed
+ * @param allocationStack stack trace that allocated the Resource, or
+ * null if {@link #collectAllocationStacks} is
+ * false.
+ * @see Resource#trackNonDisposed(boolean, NonDisposedReporter)
+ */
+ void onNonDisposedResource(Resource resource, NonDisposedException allocationStack);
+ }
+
+ /**
* the device where this resource was created
*/
Device device;
+ /**
+ * Used to report not disposed SWT resources, null by default
+ */
+ private static NonDisposedReporter nonDisposedReporter;
+
+ /**
+ * Set to {@code true} if leak detector should ignore not disposed resource
+ */
+ boolean nonDisposedIgnore;
+
+ /**
+ * Set to {@code true} if leak detector should create exceptions for not disposed resources
+ */
+ private static boolean collectAllocationStacks;
+
+ /**
+ * Recorded at object creation if {@link #trackNonDisposed(boolean)} was
+ * called with {@code true}, used to track resource disposal
+ */
+ private NonDisposedException allocationStack;
+
+ static {
+ final String property = System.getProperty("org.eclipse.swt.graphics.Resource.reportNonDisposed");
+ if (property != null) {
+ if (property.equalsIgnoreCase("stacks")) {
+ trackNonDisposed(true);
+ } else if (property.equalsIgnoreCase("true")) {
+ trackNonDisposed(false);
+ }
+ }
+ }
+
public Resource() {
}
@@ -52,6 +113,9 @@
if (device == null) device = Device.getDevice();
if (device == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
this.device = device;
+ if (collectAllocationStacks) {
+ allocationStack = new NonDisposedException();
+ }
}
void destroy() {
@@ -72,6 +136,17 @@
}
/**
+ * Returns the allocation stack if {@link #collectAllocationStacks} is enabled.
+ * Otherwise, will return {@code null}.
+ *
+ * @see #trackNonDisposed(boolean)
+ * @since 3.116
+ */
+public NonDisposedException getAllocationStack() {
+ return allocationStack;
+}
+
+/**
* Returns the <code>Device</code> where this resource was
* created.
*
@@ -101,4 +176,57 @@
*/
public abstract boolean isDisposed();
+@Override
+protected void finalize() {
+ if (nonDisposedIgnore) return;
+ if (nonDisposedReporter == null) return;
+ if (isDisposed()) return;
+
+ // Color doesn't really have any resource to be leaked, ignore
+ if (this instanceof Color) return;
+
+ nonDisposedReporter.onNonDisposedResource(this, allocationStack);
+}
+
+/**
+ * Enables detection of Resource objects for which {@link #dispose()} wasn't
+ * called, which means a leak of native memory and/or OS resources.
+ *
+ * @param collectAllocationStacks whether to collect allocation stacks for
+ * better reports. This is a performance
+ * tradeoff. Changing this value will only affect
+ * future objects.
+ * @param reporter object used to report detected errors. Use
+ * null to disable tracking. Setting a new
+ * reporter has an immediate effect.
+ *
+ * @see #getAllocationStack()
+ * @since 3.116
+ */
+public static void trackNonDisposed(boolean collectAllocationStacks, NonDisposedReporter reporter) {
+ nonDisposedReporter = reporter;
+ Resource.collectAllocationStacks = collectAllocationStacks;
+}
+
+/**
+ * Enables detection of Resource objects for which {@link #dispose()} wasn't
+ * called, which means a leak of native memory and/or OS resources. Uses default
+ * reporter that will print to {@link System#err}.
+ *
+ * @param collectAllocationStacks whether to collect allocation stacks for
+ * better reports. This is a performance
+ * tradeoff. Changing this value will only affect
+ * future objects.
+ *
+ * @see #getAllocationStack()
+ * @since 3.116
+ */
+public static void trackNonDisposed(boolean collectAllocationStacks) {
+ trackNonDisposed(collectAllocationStacks, (object, exception) -> {
+ System.err.println("Resource was not properly disposed: " + object);
+ if (exception != null)
+ exception.printStackTrace();
+ });
+}
+
}
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/Font.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/Font.java
index 21e63f9..c18dea0 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/Font.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/Font.java
@@ -233,6 +233,12 @@
public static Font gtk_new(Device device, long handle) {
Font font = new Font(device);
font.handle = handle;
+ /*
+ * When created this way, Font doesn't own its .handle, and
+ * for this reason it can't be disposed. Tell leak detector
+ * to just ignore it.
+ */
+ font.nonDisposedIgnore = true;
return font;
}
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Font.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Font.java
index ee6e908..bdaf885 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Font.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Font.java
@@ -269,6 +269,12 @@
public static Font win32_new(Device device, long handle) {
Font font = new Font(device);
font.handle = handle;
+ /*
+ * When created this way, Font doesn't own its .handle, and
+ * for this reason it can't be disposed. Tell leak detector
+ * to just ignore it.
+ */
+ font.nonDisposedIgnore = true;
return font;
}
diff --git a/bundles/org.eclipse.swt/META-INF/MANIFEST.MF b/bundles/org.eclipse.swt/META-INF/MANIFEST.MF
index 579190b..a8394e3 100644
--- a/bundles/org.eclipse.swt/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.swt/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@
Bundle-Name: %pluginName
Bundle-Vendor: %providerName
Bundle-SymbolicName: org.eclipse.swt; singleton:=true
-Bundle-Version: 3.115.200.qualifier
+Bundle-Version: 3.116.0.qualifier
Bundle-ManifestVersion: 2
Bundle-Localization: plugin
DynamicImport-Package: org.eclipse.swt.accessibility2
diff --git a/bundles/org.eclipse.swt/build.xml b/bundles/org.eclipse.swt/build.xml
index 2f44206..22fd01c 100644
--- a/bundles/org.eclipse.swt/build.xml
+++ b/bundles/org.eclipse.swt/build.xml
@@ -18,7 +18,7 @@
<target name="init">
<property name="plugin" value="org.eclipse.swt" />
- <property name="version.suffix" value="3.115.200" />
+ <property name="version.suffix" value="3.116.0" />
<property name="full.name" value="${plugin}_${version.suffix}" />
<property name="temp.folder" value="${basedir}/temp.folder" />
<property name="plugin.destination" value="${basedir}" />
diff --git a/bundles/org.eclipse.swt/pom.xml b/bundles/org.eclipse.swt/pom.xml
index c76244c..c92c177 100644
--- a/bundles/org.eclipse.swt/pom.xml
+++ b/bundles/org.eclipse.swt/pom.xml
@@ -19,7 +19,7 @@
</parent>
<groupId>org.eclipse.swt</groupId>
<artifactId>org.eclipse.swt</artifactId>
- <version>3.115.200-SNAPSHOT</version>
+ <version>3.116.0-SNAPSHOT</version>
<packaging>eclipse-plugin</packaging>
<properties>
diff --git a/tests/org.eclipse.swt.tests/ManualTests/org/eclipse/swt/tests/manual/Bug569752_DetectNonDisposedOsResources.java b/tests/org.eclipse.swt.tests/ManualTests/org/eclipse/swt/tests/manual/Bug569752_DetectNonDisposedOsResources.java
new file mode 100644
index 0000000..4f45913
--- /dev/null
+++ b/tests/org.eclipse.swt.tests/ManualTests/org/eclipse/swt/tests/manual/Bug569752_DetectNonDisposedOsResources.java
@@ -0,0 +1,71 @@
+/*******************************************************************************
+ * Copyright (c) 2020 Syntevo and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * Syntevo - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.swt.tests.manual;
+
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.graphics.Image;
+import org.eclipse.swt.graphics.Resource;
+import org.eclipse.swt.layout.GridData;
+import org.eclipse.swt.layout.GridLayout;
+import org.eclipse.swt.widgets.*;
+
+public class Bug569752_DetectNonDisposedOsResources {
+ public static void main(String[] args) {
+ // System.setProperty("org.eclipse.swt.graphics.Resource.reportNonDisposed", "stacks");
+ // System.setProperty("org.eclipse.swt.graphics.Resource.reportNonDisposed", "false");
+
+ final Display display = new Display();
+ final Shell shell = new Shell(display);
+ shell.setLayout(new GridLayout(1, true));
+
+ Label hint = new Label(shell, 0);
+ hint.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true));
+ hint.setText(
+ "1) Click the button below\n" +
+ "2) With patch, there's a leak report for Image"
+ );
+
+ Button chkDispose = new Button(shell, SWT.CHECK);
+ chkDispose.setText("Dispose objects");
+
+ Button btnTest = new Button(shell, SWT.PUSH);
+ btnTest.setText("Leak Image");
+ btnTest.addListener(SWT.Selection, event -> {
+ Image image = new Image(display, 10, 10);
+
+ if (chkDispose.getSelection())
+ image.dispose();
+
+ image = null;
+ System.gc();
+ });
+
+ Resource.trackNonDisposed(true, (object, exception) -> {
+ System.err.println("Snippet reporter: Resource was not properly disposed: " + object);
+ if (exception != null)
+ exception.printStackTrace();
+ });
+
+ shell.pack();
+ shell.open();
+
+ while (!shell.isDisposed()) {
+ if (!display.readAndDispatch()) {
+ display.sleep();
+ }
+ }
+
+ display.dispose();
+ }
+}
\ No newline at end of file
