On Wed, 5 Jun 2024 19:57:34 GMT, Mark Powers <mpow...@openjdk.org> wrote:
>> Hi,
>>
>> I need a review for this simple change to fix a threading problem with the
>> test. The server thread was not completing before the check occurred on the
>> main thread. The failure showed up in windows and macos, but not linux.
>> With this fix, running 100 times, windows & macos showed no failures.
>>
>> Tony
>
> test/jdk/javax/net/ssl/SSLSession/CertMsgCheck.java line 62:
>
>> 60: }
>> 61:
>> 62: throw new Exception("Failed to find expected alert: " + args[0]);
>
> Should it be "expected exception" rather than "expected alert"?
It's the TLS alert it's trying to find, the exception is the messenger
> test/jdk/javax/net/ssl/templates/TLSBase.java line 101:
>
>> 99: if (!empty) {
>> 100: fis = new FileInputStream(System.getProperty("test.src",
>> "./") +
>> 101: "/" + pathToStores + "/" + keyStoreFile);
>
> My Java style guide says to break before the operator.
I have never heard that. I have always ended with a plus and I believe
everyone else does too if I'm not mistaken.
> test/jdk/javax/net/ssl/templates/TLSBase.java line 246:
>
>> 244: void done() {
>> 245: try {
>> 246: t.join(5000);
>
> I had to increase the join value of a test recently that failed with virtual
> threads.
> Try adding `--test-make-args JTREG_TEST_THREAD_FACTORY=Virtual` to your mach5
> command line.
I'll check it out.. thanks
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19553#discussion_r1628567291
PR Review Comment: https://git.openjdk.org/jdk/pull/19553#discussion_r1628567917
PR Review Comment: https://git.openjdk.org/jdk/pull/19553#discussion_r1628567988
