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.

Sign up for GitHub

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

Jump to bottom

Avoid overriding finalize() #1465

Open
bt90 opened this issue Sep 12, 2024 · 5 comments · May be fixed by #1702
Open

Avoid overriding finalize() #1465

bt90 opened this issue Sep 12, 2024 · 5 comments · May be fixed by #1702

Comments

@bt90
Copy link

bt90 commented Sep 12, 2024

Is your feature request related to a problem? Please describe.

The current implementation to make Resource.setNonDisposehandler(Consumer) work, uses finalize() under the hood:

eclipse.platform.swt/bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/Resource.java

Lines 74 to 81 in 4d68329

protected void finalize() {
if (ignoreMe) 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);
}

This method is deprecated for removal since Java 18 as described in JEP 421.

Describe the solution you'd like

We should rewrite this functionality using PhantomReferences and a daemon thread polling from the shared ReferenceQueue.

Describe alternatives you've considered

The cleaner API might also work:

https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/ref/Cleaner.html
@laeubi
Copy link
Contributor

laeubi commented Sep 12, 2024

I already made some PO to use Cleaner API instead:

this has the advantage that one still can use dispose() to clear early, wait for the GC (and probably log), support clearing of Weak/SoftReference and so on.

@jukzi
Copy link
Contributor

jukzi commented Sep 12, 2024

here is a working commit that was rejected:
https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/185578/24/bundles/org.eclipse.swt/Eclipse+SWT/common/org/eclipse/swt/graphics/Resource.java#b74

@bt90
Copy link
Author

bt90 commented Sep 12, 2024

Is there any reason why we can't let the Cleaner do the disposal, effectively making dispose() a best practice instead of a necessity?

@jukzi
Copy link
Contributor

jukzi commented Sep 12, 2024

Aa cleaner depends on GC, which has literally no guaranties to run. Resources are OS objects which need to be released otherwise the OS runs in out of resource; and- a cleaner does not run in swt thread.

@otbutz otbutz linked a pull request Jan 2, 2025 that will close this issue
Open
@otbutz
Copy link

otbutz commented Jan 2, 2025

My proposed solution: #1702

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Switch to the Cleaner API otbutz/eclipse.platform.swt
4 participants
@laeubi @bt90 @otbutz @jukzi