<AWT Dev> Review a fix for List leak

[Petr Pchelko]

Hello, Artem.

I have a number of comments about the test:
1. The test lacks a copyright header. Could you please add it. You can use any other test to get an example of a correct header.
2. Please limit the maximum heap size for the test with -Xmx option. 100mb should be enough, it's pointless to fill the whole system memory.
3. The pattern in the test's main method is quite strange. It's better to dispose the frame in a finally block
4. Lines 37-43. Normally we do not use blocks to group the code together.
5. Are you sure that you do not need to wait for a frame to actually show up on the screen so that all the peers are guaranteed to get created?
6. Do not use System.exit in the tests. Please remove lines 26 to 31.
7. You need to add @bug with the bug id in the test header.

As for you initial evaluation:
>>> When method "void AwtList::SetMultiSelect" is invoked it invokes "void
>>> AwtComponent::CreateHWnd" where m_peerObject initialized. But at this
>>> stage m_peerObject already initialized and already holds ref to java
>>> List object. So original m_peerObject is lost and ref to java List lost
>>> as well. In the fix I've added check whether m_peerObject is initialized
>>> or not.
Why is is already initialized? Does this mean that the CreateHWND is called multiple times on the same component? 
I'm not sure it's correct, because this means we are creating multiple native components for the same AWT component and leaking native HWNDs.
Where is the original reference created?

Thank you.
With best regards. Petr.

On 30 июня 2014 г., at 14:17, artem malinko <artem.malinko at oracle.com> wrote:

> Thank you, Anthony.
> 
> Yes, I think assertion won't be superfluous to prevent other bugs of same type. Here is a version with assert.
> 
> http://cr.openjdk.java.net/~mcherkas/artem/webrev.02/
> 
> On 27.06.2014 1:12, Anthony Petrov wrote:
>> Hi Artem,
>> 
>> Please configure you code editor so that it formats the code that you modify according to Java code conventions used in OpenJDK (4-spaces line indentation, a space after "if" and before "{", etc.)
>> 
>> Also, please include the bug id and synopsis to the subject line of your review requests. Please see other review threads on this mailing list for examples.
>> 
>> As for the fix itself, should we add an assertion check to the CreateHWnd() method to verify that both peer and m_peerObject refer to the same Java object in case the latter is already set?
>> 
>> -- 
>> best regards,
>> Anthony
>> 
>> On 6/26/2014 7:30 PM, artem malinko wrote:
>>> Hello, AWT Team.
>>> 
>>> Please review a fix for the issue:
>>> https://bugs.openjdk.java.net/browse/JDK-8040076
>>> The fix is available at:
>>> http://cr.openjdk.java.net/~mcherkas/artem/webrev.01/
>>> 
>>> When method "void AwtList::SetMultiSelect" is invoked it invokes "void
>>> AwtComponent::CreateHWnd" where m_peerObject initialized. But at this
>>> stage m_peerObject already initialized and already holds ref to java
>>> List object. So original m_peerObject is lost and ref to java List lost
>>> as well. In the fix I've added check whether m_peerObject is initialized
>>> or not.
>>> 
>>> Thank you.
>