RFR 8163083: SocketListeningConnector does not allow invocations with port 0

[Alex Menkov]

+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
>>      >>
>>      >>
>>      >>
>>      >
>>
>>
>>
>