RFR of JDK-8171133: java/rmi/registry/reexport/Reexport.java, there is a missing case check in createReg(..)

Roger Riggs Roger.Riggs at Oracle.com
Fri Dec 16 14:13:41 UTC 2016


Hi Hamlin,

Yes, the logic would be clearer;  and I would also remove the (now) 
redundant checking.

Roger


On 12/15/2016 9:42 PM, Hamlin Li wrote:
> Hi Roger, Daniel,
>
> Thank you for reviewing.
>
> On 2016/12/15 22:45, Roger Riggs wrote:
>> Hi Hamlin,
>>
>> If this is supposed to fix the call from line 68: then doesn't the 
>> test for reg != null
>> at line 70 already have the same effect?
> Please consider the situation: LocateRegistry.createRegistry(port) in 
> createReg(true, port) does not throw exception but just return null 
> when remoteOk is true. this case is missed by current test. Although 
> we know that should not happen by checking the product code, but the 
> test should not assume how it is implemented.
> And I think the patch will make logic more clear, maybe we could 
> remove the check of reg != null at the same time after modify the code 
> in createReg(...).
>
> Thank you
> -Hamlin
>>
>> Roger
>>
>>
>> On 12/14/2016 10:19 PM, Hamlin Li wrote:
>>> Would you please review the below patch?
>>>
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8171133
>>> webrev: http://cr.openjdk.java.net/~mli/8171133/webrev.00/
>>>
>>> java/rmi/registry/reexport/Reexport.java, there is a missing case 
>>> check in createReg(..): if LocateRegistry.createRegistry(port) 
>>> return null when port is in use.
>>>
>>> Thank you
>>> -Hamlin
>>> ------------------------------------------------------------------------ 
>>>
>>> diff -r ddd192238fcb test/java/rmi/registry/reexport/Reexport.java
>>> --- a/test/java/rmi/registry/reexport/Reexport.java    Tue Dec 13 
>>> 18:47:23 2016 -0800
>>> +++ b/test/java/rmi/registry/reexport/Reexport.java    Wed Dec 14 
>>> 19:06:40 2016 -0800
>>> @@ -105,6 +105,9 @@
>>>
>>>          try {
>>>              reg = LocateRegistry.createRegistry(port);
>>> +            if (remoteOk) {
>>> +                TestLibrary.bomb("Remote registry is up, an 
>>> Exception is expected!");
>>> +            }
>>>          } catch (Throwable e) {
>>>              if (remoteOk) {
>>>                  System.err.println("EXPECTING PORT IN USE 
>>> EXCEPTION:");
>>>
>>
>



More information about the core-libs-dev mailing list