RFR: 8265240: runtime/Thread/SuspendAtExit.java needs updating

[Daniel D.Daugherty]

On Wed, 21 Apr 2021 05:50:01 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> I'm updating the runtime/Thread/SuspendAtExit.java test:
>> 
>> - switch from java.lang.Thread.suspend() to JVM/TI SuspendThread()
>> - switch from java.lang.Thread.resume() to JVM/TI ResumeThread()
>> - switch from counter-based to time-based testing
>> - improve error checking since we're now using an API with error codes!
>> 
>> I've used this test to stress @robehn's fix for JDK-8257831 using both
>> invocation styles for 9 hours each in {fastdebug, release, slowdebug}
>> configs without any issues.
>> 
>> I've run the updated test thru Mach5 Tier[134567] testing; one timeout
>> was observed in a single Tier6 run on Win-X64. I believe this is a case of
>> a lost Thread.interrupt() call.
>
> test/hotspot/jtreg/runtime/Thread/SuspendAtExit.java line 90:
> 
>> 88:         while (System.currentTimeMillis() < start_time + (timeMax * 1000)) {
>> 89:             count++;
>> 90:             SuspendAtExit threads[] = new SuspendAtExit[N_THREADS];
> 
> Style nit pre-existing: The [] go on the type not the variable.

Good catch!

As I'm re-reading this code, it occurs to me that I really don't need to
create the array of `N_THREADS` any more  since we are now time
based and we'll create a new Thread in each loop and run it through
the gauntlet until times runs out.

What do you think about changing `SuspendAtExit[N_THREADS]`
into just a single `SuspendAtExit`?

> test/hotspot/jtreg/runtime/Thread/SuspendAtExit.java line 103:
> 
>> 101:                     // the exitSyncObj.await() call and the SuspendThread()
>> 102:                     // calls will come in during thread exit.
>> 103:                     threads[i].interrupt();
> 
> Pre-existing: why use interrupt() instead of simply calling countDown() on the latch ??

Hmmm.... I don't remember why I chose to use Thread.interrupt() instead
of countDown() when I created these tests for the Thread-SMR project.
I'll try switching.

> test/hotspot/jtreg/runtime/Thread/SuspendAtExit.java line 159:
> 
>> 157:                                                "after thread #" + i +
>> 158:                                                " has been join()'ed");
>> 159:                 }
> 
> This is unnecessary, you aren't testing the correctness of Thread.join() here. join() can't return normally if the thread is alive.

Okay. Will delete the "late" isAlive() check.

> test/hotspot/jtreg/runtime/Thread/libSuspendAtExit.cpp line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2001, 2021, Oracle and/or its affiliates. All rights reserved.
> 
> Copyright year should just be 2021.

I copied this code from another file that is "Copyright (c) 2001, 2021"
and renamed just the prefix for the suspendThread() and resumeThread()
functions. Agent_OnLoad() is a straight copy.

So I think I should keep the copyright as is.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3576