Codereview requested: 6566891 RMIConnector: map value referencing map key in WeakHashMap prevents map entry to be removed

[shanliang]

Jaroslav Bachorik wrote:
> On 08/21/2013 12:00 AM, Daniel Fuchs wrote:
>   
>> On 8/20/13 11:12 PM, shanliang wrote:
>>     
>>> Thanks Daniel and Erik for the comments, here is new version:
>>>    http://cr.openjdk.java.net/~sjiang/JDK-6566891/01/
>>>       
>> This new version looks reasonable. I guess you could dispense of the new
>> nullSubjectConnRef by just using 'null' as key in the WeakHashMap (as this
>> seems to be the way it  used to work before) - but I'm OK with your
>> proposed changes.
>>     
>
> Indeed, the change could be simplified by just using the null key.
>   
If using "null" as a key, then the pair null/weakRef will never be 
removed from WeakHashMap, even the value of weakRef is null. Of course 
this is a minor memory leak, or almost nothing, but it seems more clean 
to use nullSubjectConnRef.

Still need a reviewer.

Thanks,
Shanliang
> The rest seems fine. Thumbs up!
>
> -JB-
>
>   
>> -- daniel
>>     
>>> Erik Gahlin wrote:
>>>       
>>>> Shanliang,
>>>>
>>>> Let's say the delgationSubject is null and there is previous
>>>> connection cached (nullSubjectConn != null && nullSubjectConn.get() 
>>>> != null) so you hit:
>>>>
>>>> 2016: conn = nullSubjectConn.get();
>>>>
>>>> but before you get the object a GC occurs and clears reference, then
>>>> getConnectionWithSubject would return null.
>>>>
>>>> Shouldn't you assign to nullSubjectConn.get() to variable so the
>>>> reference is kept alive? Or it OK to return null?
>>>>         
>>> Yes, should avoid calling 2 times.
>>>       
>>>> Also, is the
>>>>
>>>> 2020: if (wr != null) {
>>>>         
>>> This line is no more there.
>>>
>>> Shanliang
>>>       
>>>> necessary?
>>>>
>>>> Thanks
>>>> Erik
>>>>
>>>> nullSibwhen you hit the second if-statement
>>>>  happens
>>>> shanliang skrev 2013-08-20 17:35:
>>>>         
>>>>> webrev: http://cr.openjdk.java.net/~sjiang/JDK-6566891/00/
>>>>>
>>>>> shanliang wrote:
>>>>>           
>>>>>> Hi,
>>>>>>
>>>>>> Please review:
>>>>>>
>>>>>> webrev:
>>>>>> http://amos.fr.oracle.com/jmgt/user/sjiang/webrevs/jdk8-6566891/00/
>>>>>> bug: https://jbs.oracle.com/bugs/browse/JDK-6566891
>>>>>>
>>>>>> I have passed JCK tests and unit tests.
>>>>>>
>>>>>> Thanks,
>>>>>> Shanliang
>>>>>>
>>>>>>             
>>     
>
>   

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20130821/3d05f917/attachment.html