RFR: 8298939: Refactor open/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.sh to jtreg java test [v3]
Stuart Marks
smarks at openjdk.org
Tue Feb 21 19:02:41 UTC 2023
On Fri, 17 Feb 2023 20:32:20 GMT, Matthew Donovan <duke at openjdk.org> wrote:
>> test/jdk/javax/rmi/ssl/SSLSocketParametersTest.java line 163:
>>
>>> 161: }
>>> 162: }
>>> 163: }
>>
>> It looks like this finally-block is intended to unexport any exported RMI service, which will let the JVM exit. However, this is somewhat fragile. If the unexportObject() call fails with some other exception, the object might remain exported, and the JVM will hang indefinitely. (This has historically been a problem with RMI tests.)
>>
>> Since the original version of the test ran each case in a separate JVM, I think it's ok to continue to do the same. The old version of the test called System.exit() along most code paths. It's somewhat odd that exit wasn't called along all code paths. Perhaps those code paths weren't taken, or if they were, the JVM exited of its own accord. In any case, I'd suggest ensuring that exit() is called along all code paths (success and failure) and keeping separate `@run main/othervm` lines in the header to run each case in its own JVM. This is at least equivalent to what the shell script based test was doing.
>>
>> As an optimization, it might be reasonable to try to run some subset of the tests in a single JVM. They can't all be run in the same JVM though, because of system properties, so this would require some complexity to support running some cases in the same JVM and some in separate JVMs. It's not clear to me whether that's warranted.
>
> I don't think all of those code paths ever executed. If `System.exit()` is called, regardless of the exit code, `make` treats it as an error:
>
> `TEST RESULT: Failed. Unexpected exit from test [exit code: 0]`
>
> I added a try/catch in `main` so that if an exception is thrown during the test, `System.exit` will be called.
My bad, I was confused about the execution environments. In the old test, jtreg invoked the shell script, which in turn invoked the Java test as a standalone Java program. In that environment it's paramount that every code path call System.exit() in order to avoid stale JVMs building up.
The new test is invoked directly by jtreg, which disallows calling System.exit(). Instead, every code path needs to result in the main() method completing somehow, either via a normal return or by throwing an exception directly or letting one propagate from some called method. So, you need to pull out the try/catch and any calls to System.exit(). Sorry about that.
If we're going to stick with a scheme where we're running separate JVM per test, there's no need to unexport any RMI service. When main() returns or throws an exception, jtreg will detect this and terminate the JVM because of /othervm, and any exported RMI services will be destroyed at that time. Thus, the finally-block in `testRmiCommunication` can simply be removed.
-------------
PR: https://git.openjdk.org/jdk/pull/11910
More information about the core-libs-dev
mailing list