One more note:
please add "@Override" annotation to the
SocketListeningConnector.updateArgumentMapIfRequired
--alex
On 09/21/2018 02:26, gary.ad...@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
