Hello, Artem. 

Please have a look to the new version of this fix:
http://cr.openjdk.java.net/~pchelko/7079260/webrev.02/

As we have discussed offline, I've replaced the needResetXICClient reference to 
the weak ref. The second reference clientComponent in the 
CompositionAreaHandler is used quite similar to the needResedXICClient field, 
so I thought it is OK to use a Weak Ref there too, as it makes the code much 
cleaner.

With best regards. Petr.

On Feb 4, 2013, at 4:52 PM, Artem Ananiev wrote:

> Hi, Petr,
> 
> I looked at the changes again and noticed that we now have two methods in 
> InputMethodAdapter that look quite related to each other:
> 
> protected void cleanClient(Component)
> 
> and
> 
> void setClientComponent(Component)
> 
> In the only class where cleanClient() is overridden, cleanClient is often the 
> same as needResetXICClient. All the above makes me believe we can fix the 
> leak without introducing cleanClient(), but making setClientComponent() 
> protected and using it in subclasses.
> 
> Thanks,
> 
> Artem
> 
> On 2/1/2013 6:45 PM, Petr Pchelko wrote:
>> Hello, Artem.
>> 
>> Sorry for the delay, I've got distracted by other issues.
>> 
>> The new version of the fix is available at:
>> http://cr.openjdk.java.net/~pchelko/7079260/webrev.01/
>> 
>>> 1. In JTextComponent you need to override addNotify() to re-install caret 
>>> change listener.
>> 
>> I have all removed the changes in JTextComponent which were needed to clean 
>> the caret. There are rootset references from the caret timer thread to the 
>> component, however, these references are cleaned up on the next blink of the 
>> caret. I've tried to clean the caret on removeNotify and restore it's state 
>> on addNotify, however it does not seem possible without changes in public 
>> APIs and without adding some unnecessary fields.
>> 
>> So, as this is a very short-timed memory leak and the component will 
>> certainly be cleaned up it half a second.
>> 
>>> X11InputMethod can't be referenced in sun.awt.im code (in fact, X11* 
>>> classes are absent on Windows at all).
>> Done.
>> 
>> With best regards. Petr.
>> 
>> On Jan 18, 2013, at 5:22 PM, Artem Ananiev wrote:
>> 
>>> 
>>> Minor comments:
>>> 
>>> 1. In JTextComponent you need to override addNotify() to re-install caret 
>>> change listener.
>>> 
>>> 2. X11InputMethod can't be referenced in sun.awt.im code (in fact, X11* 
>>> classes are absent on Windows at all).
>>> 
>>> Thanks,
>>> 
>>> Artem
>>> 
>>> On 1/18/2013 4:26 PM, Petr Pchelko wrote:
>>>> Hello, this is a reminder.
>>>> 
>>>> For your convenience:
>>>> 
>>>> 7079260 : InputContext leaks memory
>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7079260
>>>> 
>>>> The new webrev is available here:
>>>> http://cr.openjdk.java.net/~serb/petr/7079260/webrev/
>>>> 
>>>> With best regards. Petr.
>>>> 
>>>> On Jan 10, 2013, at 12:49 PM, Petr Pchelko wrote:
>>>> 
>>>>> Hello.
>>>>> 
>>>>> Sorry, I've forgot about licenses. I will add them before the push.
>>>>> 
>>>>> With best regards. Petr.
>>>>> 
>>>>> On Jan 9, 2013, at 4:38 PM, Petr Pchelko wrote:
>>>>> 
>>>>>> Hello, here is the new version of the fix with the test attached.
>>>>>> 
>>>>>> While writing the test I have noticed some additional references which 
>>>>>> also were not removed and could lead to a memory leak, so now the 
>>>>>> following references are cleaned:
>>>>>> 1. References from X11InputMethod which were reported in an original CR
>>>>>> 2. References from CompositionAreaHandler
>>>>>> 3. References from the Caret timer. It is not critical, as these 
>>>>>> references were removed at the time of the next caret blink, however now 
>>>>>> it is cleaned immediately.
>>>>>> 
>>>>>> The new webrev is available here:
>>>>>> http://cr.openjdk.java.net/~serb/petr/7079260/webrev/
>>>>>> 
>>>>>> Best, Petr.
>>>>>> 
>>>>>> On Dec 21, 2012, at 7:16 PM, Sergey Bylokhov wrote:
>>>>>> 
>>>>>>> Hi, Petr.
>>>>>>> It would be good to have appropriate testcase for this issue too.
>>>>>>> 
>>>>>>> 21.12.2012 16:57, Petr Pchelko wrote
>>>>>>>> Hello,
>>>>>>>> 
>>>>>>>> Could you please review the fix for the issue:
>>>>>>>> 7079260 : InputContext leaks memory
>>>>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7079260
>>>>>>>> 
>>>>>>>> The webrev is available at:
>>>>>>>> http://cr.openjdk.java.net/~art/pchelko/7079260/webrev/
>>>>>>>> 
>>>>>>>> The memory leak component in the test, provided in the description of 
>>>>>>>> the bug is still now collected with this fix, however now all the 
>>>>>>>> references are in netbeans code, not AWT.
>>>>>>>> 
>>>>>>>> The fix was tested on Linux platform with toy apps and automatic tests 
>>>>>>>> related to im.
>>>>>>>> 
>>>>>>>> Best, Petr.
>>>>>>> 
>>>>>>> 
>>>>>>> --
>>>>>>> Best regards, Sergey.
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>> 

Reply via email to