RFR 8163083: SocketListeningConnector does not allow invocations with port 0

[JC Beyler]

Looks good to me Daniil (not a reviewer though),

Thanks!
Jc

On Fri, Sep 21, 2018 at 8:32 AM Daniil Titov <daniil.x.titov at oracle.com>
wrote:

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

-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180921/28f39190/attachment.html>