From f8015ff2772b72c19d6cec9f5cc65c82f6348632 Mon Sep 17 00:00:00 2001 From: otbutz Date: Thu, 2 Jan 2025 13:04:18 +0100 Subject: [PATCH 1/3] Switch to the Cleaner API --- .../org/eclipse/swt/graphics/Resource.java | 56 ++++++++++--------- 1 file changed, 29 insertions(+), 27 deletions(-) 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 5d61855544b..a580fa4c100 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 @@ -13,7 +13,9 @@ *******************************************************************************/ package org.eclipse.swt.graphics; +import java.lang.ref.*; import java.util.*; +import java.util.concurrent.atomic.*; import java.util.function.*; import org.eclipse.swt.*; @@ -44,41 +46,37 @@ public abstract class Resource { /** - * Used to track not disposed SWT resource. A separate class allows - * not to have the {@link #finalize} when tracking is disabled, avoiding - * possible performance issues in GC. + * Used to track not disposed SWT resource. */ - private static class ResourceTracker { - /** - * Resource that is tracked here - */ - private Resource resource; - + private static class ResourceTracker implements Runnable { /** * Recorded at Resource creation if {@link #setNonDisposeHandler} was * enabled, used to track resource disposal */ - private Error allocationStack; + private final Error allocationStack; /** - * Allows to ignore specific Resources even if they are not disposed - * properly, used for example for Fonts that SWT doesn't own. + * Allows to disarm this cleaning action. */ - boolean ignoreMe; + private final AtomicBoolean ignore = new AtomicBoolean(false); - ResourceTracker(Resource resource, Error allocationStack) { - this.resource = resource; + ResourceTracker(Error allocationStack) { this.allocationStack = allocationStack; } + /** + * Causes all future invocations of {@link #run()} to be ignored + */ + public void ignore() { + ignore.set(true); + } + @Override - protected void finalize() { - if (ignoreMe) return; + public void run() { + if (ignore.get()) return; if (nonDisposedReporter == null) return; - // If the Resource is GC'ed before it was disposed, this is a leak. - if (!resource.isDisposed()) - nonDisposedReporter.accept(allocationStack); + nonDisposedReporter.accept(allocationStack); } } @@ -87,6 +85,11 @@ protected void finalize() { */ Device device; + /** + * Invokes {@link ResourceTracker#run()} once the resource is eligible for GC + */ + private static final Cleaner cleaner = Cleaner.create(); + /** * Used to report not disposed SWT resources, null by default */ @@ -142,6 +145,7 @@ void destroyHandlesExcept(Set zoomLevels) { * This method does nothing if the resource is already disposed. */ public void dispose() { + if (tracker != null) tracker.ignore(); if (device == null) return; if (device.isDisposed()) return; destroy(); @@ -165,7 +169,7 @@ public Device getDevice() { void ignoreNonDisposed() { if (tracker != null) { - tracker.ignoreMe = true; + tracker.ignore(); } } @@ -177,17 +181,15 @@ void initNonDisposeTracking() { // Color doesn't really have any resource to be leaked, ignore. if (this instanceof Color) return; - // Avoid performance costs of having '.finalize()' when not tracking. + // Avoid performance costs of gathering the current stack trace when not tracking. if (nonDisposedReporter == null) return; // Capture a stack trace to help investigating the leak Error error = new Error("SWT Resource was not properly disposed"); //$NON-NLS-1$ - // Allocate a helper class with '.finalize()' in it, it will do the actual - // work of detecting and reporting errors. This works because Resource - // holds the only reference to 'ResourceTracker' and therefore the tracker - // is only GC'ed when Resource itself is ready to be GC'ed. - tracker = new ResourceTracker(this, error); + // Create and register the tracker to run as a cleaning action for this resource. + tracker = new ResourceTracker(error); + cleaner.register(this, tracker); } /** From 81ee85621dbb21009680ee44c75d850186d792e3 Mon Sep 17 00:00:00 2001 From: Thomas Butz Date: Thu, 2 Jan 2025 14:18:58 +0100 Subject: [PATCH 2/3] Enable reporting in init --- .../org/eclipse/swt/graphics/Resource.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) 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 a580fa4c100..04670d55338 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 @@ -58,22 +58,23 @@ private static class ResourceTracker implements Runnable { /** * Allows to disarm this cleaning action. */ - private final AtomicBoolean ignore = new AtomicBoolean(false); + private final AtomicBoolean reporting = new AtomicBoolean(false); ResourceTracker(Error allocationStack) { this.allocationStack = allocationStack; } /** - * Causes all future invocations of {@link #run()} to be ignored + * @param value whether the {@link Resource#nonDisposedReporter} should be + * notified */ - public void ignore() { - ignore.set(true); + public void setReporting(boolean value) { + reporting.set(value); } @Override public void run() { - if (ignore.get()) return; + if (!reporting.get()) return; if (nonDisposedReporter == null) return; nonDisposedReporter.accept(allocationStack); @@ -145,7 +146,7 @@ void destroyHandlesExcept(Set zoomLevels) { * This method does nothing if the resource is already disposed. */ public void dispose() { - if (tracker != null) tracker.ignore(); + if (tracker != null) tracker.setReporting(false); if (device == null) return; if (device.isDisposed()) return; destroy(); @@ -169,12 +170,13 @@ public Device getDevice() { void ignoreNonDisposed() { if (tracker != null) { - tracker.ignore(); + tracker.setReporting(false); } } void init() { if (device.tracking) device.new_Object(this); + if (tracker != null) tracker.setReporting(true); } void initNonDisposeTracking() { From b6b5f9e62b9c1b1872925ea0d88e0b73335849bc Mon Sep 17 00:00:00 2001 From: Thomas Butz Date: Fri, 3 Jan 2025 08:44:52 +0100 Subject: [PATCH 3/3] Register tracker during init() --- .../org/eclipse/swt/graphics/Resource.java | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) 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 04670d55338..75223b4a92f 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 @@ -56,7 +56,7 @@ private static class ResourceTracker implements Runnable { private final Error allocationStack; /** - * Allows to disarm this cleaning action. + * Controls whether the {@link Resource#nonDisposedReporter} should be notified */ private final AtomicBoolean reporting = new AtomicBoolean(false); @@ -64,14 +64,6 @@ private static class ResourceTracker implements Runnable { this.allocationStack = allocationStack; } - /** - * @param value whether the {@link Resource#nonDisposedReporter} should be - * notified - */ - public void setReporting(boolean value) { - reporting.set(value); - } - @Override public void run() { if (!reporting.get()) return; @@ -146,7 +138,7 @@ void destroyHandlesExcept(Set zoomLevels) { * This method does nothing if the resource is already disposed. */ public void dispose() { - if (tracker != null) tracker.setReporting(false); + if (tracker != null) tracker.reporting.set(false); if (device == null) return; if (device.isDisposed()) return; destroy(); @@ -170,13 +162,15 @@ public Device getDevice() { void ignoreNonDisposed() { if (tracker != null) { - tracker.setReporting(false); + tracker.reporting.set(false); } } void init() { if (device.tracking) device.new_Object(this); - if (tracker != null) tracker.setReporting(true); + if (tracker != null && tracker.reporting.compareAndSet(false, true)) { + cleaner.register(this, tracker); + } } void initNonDisposeTracking() { @@ -189,9 +183,8 @@ void initNonDisposeTracking() { // Capture a stack trace to help investigating the leak Error error = new Error("SWT Resource was not properly disposed"); //$NON-NLS-1$ - // Create and register the tracker to run as a cleaning action for this resource. + // Create the tracker that will later be registered as a cleaning action for this resource. tracker = new ResourceTracker(error); - cleaner.register(this, tracker); } /**