On 29.04.16 15:53, Anton Tarasov wrote:
It seems so. But that might be not that critical, because it doesn’t hold (it
won’t) any UI controls and all the UI tree. Anyway it leaks, yes.
Looks like this is crossplatform bug and it also affects d3d/ogl. So
probably we can fix it on the upper level? validatedSrc/Dst/Data is
stored the surfaces which were validated and ready to paint. from the
first point of view we can change them to soft reference, and take care
about null values. Note that such changes are 2d related code and should
be reviewed on 2d-dev.
Assigned the peer/surfaceData to null in CGLayer can causes an NPE in all its
usage, because there is no any synchronization which will prevent the usage of
CGLayer after disposing.
That’s bad. Will wrapping the refs with AtomicReference help?
Unrelated to the fix, but it seems we should call flush() on surface when the
layer is disposed, at least I do not understand where we flush the native ogl
data for the latest surface data.
This will trigger CGLLayerSurfaceData.invalidate(), but the “layer” will still
not be nullified. What about nullifying it in invalidate()? Will we face the
same synchronisation issue?
Anton.
On 29.04.16 15:00, Anton Tarasov wrote:
Hi Sergey, Alexander,
Please review the fix:
bug: JDK-8028486 [TEST_BUG] [macosx]
java/awt/Window/WindowsLeak/WindowsLeak.java fails
webrev: http://cr.openjdk.java.net/~ant/JDK-8028486/webrev.0
I’m copying my comment from CR:
Please open the attached screenshot [*], made with YourKit, where a chain of
links is shown from the GC roots.
The frame is held by its peer which is held by CGLLayer which is held as
validatedSrcData in the GL context.
The point is that the GL context doesn't cleanup the last state until under
some conditions, which are not applicable to this scenario.
I'm not sure should the cleanup be triggered here or not, but the problem can
be solved otherwise.
The point is that in the chain the CGLLayer instance has been disposed, in
response to the frame disposal.
So, this is the only ref that holds it (the JNI ref is released by the native
peer on disposal).
Thus, as the layer is disposed it can at least zero all the java refs it holds
(this change already fixes the problem).
Then, the "layer" ref in CGLLayerSurfaceData should probably be made weak.
[*] https://bugs.openjdk.java.net/secure/attachment/59121/8028486.png
As to the “weak ref” mentioned in the comment. I didn’t do that, but if you
find it reasonable I can add that change (or file a separate CR).
Also, the fix contains some additional cleanup (not related to this CR): two
more JNI local refs leak, fixed.
Thanks,
Anton.
--
Best regards, Sergey.
--
Best regards, Sergey.
