-
Notifications
You must be signed in to change notification settings - Fork 185
Track and dispose resources created by operations in GC #2328
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
Track and dispose resources created by operations in GC #2328
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, the proposal looks great. Maybe can further improve the temporary resource handling in a follow-up by only storing the resource's data and creating new resources only temporarily on demand, but this already properly addresses potentialy resource leaks, so let's start with this proposal.
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java
Outdated
Show resolved
Hide resolved
c61d1a8 to
3250be3
Compare
3250be3 to
28f8835
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the code. For all the Operations, a disposable resource is registered, are all disposed on the disposal of the GC. However, I have one concern over Region.
For Region, we do not store the region object but the handle for the zoom. Which means if the GC is refreshed with a different zoom, the same handle will be used. Probably not related to this PR, but maybe we need to check if we would run in a problem with that.
Looks good. Approving.
Each Operation subclass now maintains its own list of disposable SWT resources. These are added to a `disposables` list within the Operation and are disposed when the GC is disposed. This ensures cleanup of resources.
28f8835 to
999d3db
Compare
Description:
Each Operation subclass now maintains its own list of disposable SWT resources. These are added to a
disposableslist within the Operation and are disposed when the GC is disposed. This ensures cleanup of resources.Steps to Reproduce:
There is currently an issue with SWT resource tracking due to monitor-specific scaling. To reproduce the warning:
1)Temporarily comment out
resourcesWithZoomSupport.add(resource);in the Device class (i.e., the implementation of registerResourceWithZoomSupport):2)Start the Eclipse runtime workspace with the following VM argument:
-Dorg.eclipse.swt.graphics.Resource.reportNonDisposed=true3)Wait a minute for the garbage collector to run.
You will then see the following warning in the logs: