RFR 8163083: SocketListeningConnector does not allow invocations with port 0

[Daniil Titov]

Thank you, JC!

Please review an updated version of the patch that addresses these issues.

Bug: https://bugs.openjdk.java.net/browse/JDK-8163083  
Webrev: http://cr.openjdk.java.net/~dtitov/8163083/webrev.03/

Best regards,
Daniiil

From: JC Beyler <jcbeyler at google.com>
Date: Thursday, September 20, 2018 at 8:59 PM
To: <daniil.x.titov at oracle.com>
Cc: <serviceability-dev at openjdk.java.net>
Subject: Re: RFR 8163083: SocketListeningConnector does not allow invocations with port 0

Hi Daniil,

I noticed a few nits:

http://cr.openjdk.java.net/~dtitov/8163083/webrev.02/test/hotspot/jtreg/vmTestbase/nsk/jdi/ListeningConnector/supportsMultipleConnections/supportsmultipleconnections002.java.html

  -> You have two 2018 in the copyright

http://cr.openjdk.java.net/~dtitov/8163083/webrev.02/src/jdk.jdi/share/classes/com/sun/tools/jdi/SocketListeningConnector.java.udiff.html

 -> Both new methods have an empty line at the start, is that intended?
 -> updateArgumentMapIfRequired: you check the length > 0 but assume actually it is > 1

http://cr.openjdk.java.net/~dtitov/8163083/webrev.02/src/jdk.jdi/share/classes/com/sun/tools/jdi/GenericListeningConnector.java.udiff.html
  -> Same not sure we need the extra line

Apart from that it looks good to me.

Thanks,
Jc

On Thu, Sep 20, 2018 at 8:00 PM Daniil Titov <mailto:daniil.x.titov at oracle.com> 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





-- 

Thanks,
Jc