[OpenJDK 2D-Dev] bug 8146238 - Java Queue Flusher on MacOS

[Phil Race]

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.
>>>
>>>
>>
>
>