The fix creates a new JNI GlobalRef pointing to the GraphicsConfig
That is then explicitly released in the dispose method of the SurfaceData
called when it is collected. The dispose method also then frees the native
component now that we are truly done with it
So the risk here is that I have been trying to look for is to be sure
that we don't
have any reference back to the SurfaceData from the GraphicsConfig
since it would never get collected in such a case.
So far I think we are safe. Did you also check for this ?
I have built and tested this doing add/remove monitors,
sleep/wake, switch users and no crashes as yet.
-phil.
On 8/12/19 2:29 AM, Sergey Bylokhov wrote:
Hi, Phil, Alexey.
I recheck this bug, and here is some of my thought:
1. We have two java classes:
- GLX/CGL/WGL/GraphicsConfig which maintain the native structure
WGL/GLX/CGL/GraphicsConfigInfo
- GLX/CGL/WGL/SurfaceData which maintain the native structure
CGL/GLXS/WGL/SDOps
2. The native structures should be disposed of by the "Disposer"
machinery
3. The native part XXXSDOps has a pointer to the CGL/GraphicsConfigInfo
4. To prevent the usage of dangling pointer to the
XXXGraphicsConfigInfo, the java part of XXXSDOps has a strong
reference to the java part of XXXGraphicsConfigInfo
The assumption at point 4 is not correct, it is possible that the
native part of the XXXGraphicsConfigInfo could be disposed before
XXXSDOps, and we will get a crash when we will try to dispose the
XXXSDOps.
Long time ago when it was implemented it works fine, because we never
dispose the graphics configs, we read them on-start and use till the
end of the application, even now we use this approach on
Linux(JDK-8076313).
So this bug existed on Linux, Windows and migrated to the macOS
platform, but only on macOS and windows we can get a crash.
I have rechecked the usage of the pointer from XXXSDOps to
XXXGraphicsConfigInfo which caused a crash and think that we can get
rid it, but it will required to change bunch of code on all platforms,
so as a minimal fix I suggest this one:
http://cr.openjdk.java.net/~serb/8146238/webrev.00
Just to store the reference to the GC till the moment it will not be
used by the SurfaceData.
On 8/7/19 10:56 am, Phil Race wrote:
Sergey,
This fix seems OK to me. Can you please do whatever re-evaluation you
meant
as I'd like to pull it into jdk/client for Alexey (since he does not
have current commit rights).
-phil.
On 3/8/18 2:59 PM, Sergey Bylokhov wrote:
Hi, Bill.
Thank you for confirmation.
On 08/03/2018 14:08, Bill York wrote:
3. Is there a plan to get this bug fix into the JRE distributed by
Oracle?
I will reevaluate the fix for inclusion in jdk11.
