RFR 8163083: SocketListeningConnector does not allow invocations with port 0

[Alex Menkov]

Daniil,

Just remembered that SQE requested to not add new tests to vmTestbase 
(see test/hotspot/jtreg/vmTestbase/README.md)
Could you please move the test to correct location (I suppose it's 
test/jdk/com/sun/jdi)

--alex


On 09/24/2018 14:30, Alex Menkov wrote:
> +1
> 
> --alex
> 
> On 09/24/2018 10:39, Gary Adams wrote:
>> Looks good to me.
>>
>> On 9/24/18, 1:25 PM, Daniil Titov wrote:
>>> Hi Alex and Gary,
>>>
>>> Please review the updated patch that includes suggested changes.
>>>
>>> http://cr.openjdk.java.net/~dtitov/8163083/webrev.04/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8163083
>>>
>>> Thanks!
>>> --Daniil
>>>
>>>
>>>
>>>
>>> On 9/21/18, 3:59 PM, "Alex Menkov"<alexey.menkov at oracle.com>  wrote:
>>>
>>>      One more note:
>>>      please add "@Override" annotation to the
>>>      SocketListeningConnector.updateArgumentMapIfRequired
>>>
>>>      --alex
>>>
>>>      On 09/21/2018 02:26, gary.adams at oracle.com wrote:
>>>      >  Looks good to me.
>>>      >
>>>      >  For the javadoc
>>>      >
>>>      >     72      *<p>
>>>      >     73      * If<code>arguments</code>  contains addressing 
>>> information. and
>>>      >     74      * only one connection will be accepted, the {@link 
>>> #accept accept} method
>>>      >     75      * can be called immediately without calling this 
>>> method.
>>>      >     76      *
>>>      >  77 * If the addressing information provided 
>>> in<code>arguments</code>
>>>      >  implies
>>>      >  78 * the auto detection this information might be updated 
>>> with the address
>>>      >  79 * of the started listener.
>>>      >
>>>      >     - you could add a<p>  tag if you want to maintain the 
>>> spacing in the
>>>      >  generated javadoc.
>>>      >     - I've seen other changesets migrating to {@code ..} from 
>>> the older
>>>      >  <code>...</code>
>>>      >
>>>      >  It would be good to include some negative testing.
>>>      >  Not sure if it is already covered in other tests, e.g.
>>>      >
>>>      >       args1 = defaultArguments()
>>>      >       startListening(args1)   // bound port updated
>>>      >       startListening(args1)   // already listening
>>>      >
>>>      >
>>>      >  On 9/20/18 10:59 PM, Daniil Titov wrote:
>>>      >>  Please review the change that fixes the issue in 
>>> com.sun.tools.jdi.SocketListenerConnector.startListening() method.
>>>      >>
>>>      >>  When the argument map passed to startListening() methods has 
>>> the port number unspecified or set to zero the port is auto detected. 
>>> However,  the consequent call of startListening() methods with 
>>> unspecified port number fails rather than starts a new listener on 
>>> auto detected port. This happens since the original argument map 
>>> passed to the startListening() methods is used as a key to store the 
>>> mapping to the started listeners.
>>>      >>
>>>      >>  The fix ensures that in cases when the port is auto detected 
>>> the argument map is updated with the bound port number.
>>>      >>
>>>      >>  Mach5 vmTestbase_nsk_jdi tests successfully passed.
>>>      >>
>>>      >>  Bug:https://bugs.openjdk.java.net/browse/JDK-8163083
>>>      >>  Webrev:http://cr.openjdk.java.net/~dtitov/8163083/webrev.02/
>>>      >>
>>>      >>  Thanks!
>>>      >>  --Daniil
>>>      >>
>>>      >>
>>>      >>
>>>      >
>>>
>>>
>>>
>>