Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to the Cleaner API #1702

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand Down Expand Up @@ -44,41 +46,38 @@
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 reporting = new AtomicBoolean(false);

ResourceTracker(Resource resource, Error allocationStack) {
this.resource = resource;
ResourceTracker(Error allocationStack) {
this.allocationStack = allocationStack;
}

/**
* @param value whether the {@link Resource#nonDisposedReporter} should be
* notified
*/
public void setReporting(boolean value) {
reporting.set(value);
}

@Override
protected void finalize() {
if (ignoreMe) return;
public void run() {
if (!reporting.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);
}
}

Expand All @@ -87,6 +86,11 @@ protected void finalize() {
*/
Device device;

/**
* Invokes {@link ResourceTracker#run()} once the resource is eligible for GC
*/
private static final Cleaner cleaner = Cleaner.create();
Copy link
Author

Choose a reason for hiding this comment

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

We can pass our own ThreadFactory to have a nicer thread name, but IMHO this adds too much complexity for what it's worth.


/**
* Used to report not disposed SWT resources, null by default
*/
Expand Down Expand Up @@ -142,6 +146,7 @@ void destroyHandlesExcept(Set<Integer> zoomLevels) {
* This method does nothing if the resource is already disposed.
*/
public void dispose() {
if (tracker != null) tracker.setReporting(false);
if (device == null) return;
if (device.isDisposed()) return;
destroy();
Expand All @@ -165,29 +170,28 @@ public Device getDevice() {

void ignoreNonDisposed() {
if (tracker != null) {
tracker.ignoreMe = true;
tracker.setReporting(false);
}
}

void init() {
if (device.tracking) device.new_Object(this);
if (tracker != null) tracker.setReporting(true);
}

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);
otbutz marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
Loading