[OpenJDK 2D-Dev] [9] Review Request: 8028486 java/awt/Window/WindowsLeak/WindowsLeak.java fails

[Sergey Bylokhov]

On 03.05.16 6:37, Philip Race wrote:
> On 5/2/16, 2:28 PM, Sergey Bylokhov wrote:
>> On 03.05.16 0:07, Phil Race wrote:
>>>> And this is fine because we use only identity checks inside the
>>>> validate method. If the java object was deleted we should update the
>>>> native state anyway.
>>>
>>> Not sure how to read that. Do you mean there is always
>>> some amount of work to do anyway ?
>>
>> An example: if we started the paint operation for some surface and set
>> the clip, then for the next paint operation we skip setting the
>> surface and clip if they "==" to the previous values. So if the old
>> data were deleted by GC will mean that we cannot draw to them(and
>> these cached references are not useful)
>>
>
> So that is a yes .. this is an incidental amount of work ?

I am not sure what you mean by "incidental amount of work"? Can you 
please clarify.

>
>>>
>>> Is there any value in doing the "nulling" in Anton's earlier version
>>> of the fix to be more prompt ?
>>
>> The current change is one step-up in the chain of references, and the
>> nulling in the CGlayer code will leave the leak of surface.
>
>
> I know it is not sufficient, but is it a worthwhile addition ?

If we will set some data to null, then we will need to update all usage 
of it to be ready to null. I think that the current solution cover the 
problem (but there are some other leaks which will be fixed separately).

>>
>>> is BufferedContext only used by D3D & OGL ?
>>
>> Yes, but I checked direct usage.
>
> What does direct usage mean ?
> The test program does not do this.

I meant that, I grep all usages of this class, and all of these usages 
in d3d/ogl.

> Sounds good.

I complete full jck/regression testrun(win+d3d), no new bugs were found.

>
> -phil.
>
>>
>>>>> On 05/02/2016 01:26 PM, Sergey Bylokhov wrote:
>>>>>> Hello,
>>>>>> Please review the fix for jdk9.
>>>>>>
>>>>>> Bug evaluation was done by Anton:
>>>>>> http://mail.openjdk.java.net/pipermail/awt-dev/2016-April/011177.html
>>>>>>
>>>>>> This is a cross-platform bug it affects d3d/ogl pipelines. The
>>>>>> problem
>>>>>> is that BufferedContex cached information to skip some native
>>>>>> reconfigurations. But this cache cause a memory leak if some
>>>>>> data(src/dst surfaces) was cached and there was no new rendering in
>>>>>> this context(we create context per-d3d_device/ogl_config).
>>>>>>
>>>>>> In the fix I changed all these caches to weak references. Note that i
>>>>>> use a references as initial values instead of null, just to eliminate
>>>>>> the null checks in the body of the method.
>>>>>>
>>>>>> The test was updated to be more stable(flushed the EDT + flushed the
>>>>>> Disposer thread).
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8028486
>>>>>> Webrev can be found at:
>>>>>> http://cr.openjdk.java.net/~serb/8028486/webrev.00
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>


-- 
Best regards, Sergey.