<AWT Dev> [9] Request for review: JDK-8145984 sun.lwawt.macosx.CAccessible leaks

[Sergey Bylokhov]

Hi, Anton.
On 10.01.16 13:12, Anton Tarasov wrote:
>
> http://cr.openjdk.java.net/~ant/JDK-8145984/jdk8u/webrev.2/
>
> and the discussion with Sergey.

I am a little bit worried about the changes in CGLLayer. Is it possible 
that javaLayer will be collected in native when we tries to use it?

>
>> On 04 Jan 2016, at 21:16, Pete Brunet <peter.brunet at oracle.com
>> <mailto:peter.brunet at oracle.com>> wrote:
>>
>> Hi Anton, Why did you change CAccessible.dispose() from protected to
>> public?  Shouldn't it be left protected?  -Pete
>
> I did that due to introducing an interface with dispose() method. But
> with the latest fix, there’s no that change anymore.
>
> Regards,
> Anton.
>
>>
>> On 12/22/15 3:45 PM, Anton Tarasov wrote:
>>> Hi Pete,
>>>
>>> Thanks for the review!
>>>
>>>> On 22 Dec 2015, at 23:07, Pete Brunet
>>>> <<mailto:peter.brunet at oracle.com>peter.brunet at oracle.com> wrote:
>>>>
>>>> Hi Anton, Some comments/questions:
>>>> - Some copyright dates need updating
>>>
>>> Indeed, I’ll update them.
>>>
>>>> - Line 1112 of JavaComponentAccessibility: does the release of
>>>> jaccessible cause a release of jparent?
>>>
>>> As I can see, jparent there is only a ref to jComponent, which in its
>>> turn is a JNIGlobalRef (or anyway is a class field). So, I don’t see
>>> the need to delete it… (or did I miss something?)
>>>
>>>> - Line 7155 of Component.java: is that the only place where this
>>>> means is needed?
>>>
>>> If you mean to call AC.dispose() than - yes, I think so. We rather
>>> don’t want to dispose the context until the Component goes out of the
>>> UI hierarchy, which is when Component.removeNotify() is _always_
>>> getting called (for hw & lw components).
>>>
>>> Anton.
>>>
>>>>
>>>> Pete
>>>>
>>>> On 12/22/15 8:10 AM, Anton Tarasov wrote:
>>>>> Hi guys!
>>>>>
>>>>> Could you please review the problem I’ve filed and the suggested fix?
>>>>>
>>>>> bug: JDK-8145984
>>>>> <https://bugs.openjdk.java.net/browse/JDK-8145984> sun.lwawt.macosx.CAccessible
>>>>> leaks
>>>>> webrev: http://cr.openjdk.java.net/~ant/JDK-8145984/jdk9/webrev.0
>>>>> <http://cr.openjdk.java.net/%7Eant/JDK-8145984/jdk9/webrev.0>
>>>>>
>>>>> (This is to be addressed in 8u/9. The webrev for 8u is in JIRA,
>>>>> it’s identical except the paths.)
>>>>>
>>>>> Please, find the details in JIRA.
>>>>>
>>>>> Thanks!
>>>>> Anton.
>>>>
>>>
>>
>


-- 
Best regards, Sergey.