On Fri, 17 Feb 2023 19:33:04 GMT, Stuart Marks <sma...@openjdk.org> wrote:
>> Matthew Donovan has updated the pull request incrementally with one >> additional commit since the last revision: >> >> added exceptions for cases 4 and 5 > > 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. ------------- PR: https://git.openjdk.org/jdk/pull/11910
