On Wed, 4 Jan 2023 13:04:10 GMT, Matthias Baesken <mbaes...@openjdk.org> wrote:
>> The test serviceability/sa/sadebugd/SADebugDTest.java can pass under some
>> circumstances a negative rmiport (--rmiport -1) to SALauncher.java.
>> This leads to a somewhat misleading message
>> `[debugd] Argument is expected for 'rmiport' `
>> (we set an argument [-1] but probably this is not what is really expected)
>> and additionally the real exception is not shown.
>> Probably also a warning in case of negative rmiport values should be printed
>> because they seem to lead to errors.
>
> Matthias Baesken has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Adjust Copyright years, omit one output line
The changes here to log a better error are I think good (maybe adding the
stacktrace printing is not needed now the error is more understandable!), but
aborting the launch attempt in the caller, if it gets -1 when trying to find a
free port, should be even clearer?
In
test/hotspot/jtreg/serviceability/sa/sadebugd/SADebugDTest.java
the method testWithPid() finds an rmiPort:
final int rmiPort = useRmiPort ?
Utils.findUnreservedFreePort(REGISTRY_DEFAULT_PORT, registryPort) : -1;
...but can proceed to launch even if it gets that -1 value? That looks like a
mistake.
In the "if (useRmiPort) {" block, we should be failing the test if rmiPort is
-1, saying something like
"cannot find an rmiPort, findUnreservedFreePort returns -1"
A similar abort if setting registryPort gets -1 might also be good?
-------------
PR: https://git.openjdk.org/jdk/pull/11811
