RFR 8163083: SocketListeningConnector does not allow invocations with port 0
Gary Adams
gary.adams at oracle.com
Mon Sep 24 17:39:38 UTC 2018
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
> >>
> >>
> >>
> >
>
>
>
More information about the serviceability-dev
mailing list