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

[Daniel Fuchs]

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.

-- 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/6180de4f/attachment.html