RFR: 8243455: Many SA tests can fail due to trying to get the stack trace of an active method [v2]

[Yasumasa Suenaga]

On Tue, 9 Mar 2021 07:27:26 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> Many tests run `LingeredApp`, get its stack with `jstack`, and then look for `LingeredApp.main` in the output, which should appear in the stack trace of the main thread. Usually this works fine since the main thread is in a loop, and usually blocked in `Thread.sleep()`. However, during the short period of time it wakes up from sleep, the thread's stack might not be walk-able, in which case the `LingeredApp.main` frame will not appear, and the tests looking for it will fail.
>> 
>> The fix is to introduce a new thread called `SteadyStateThread` that has a method called `steadyState()` that blocks indefinitely trying to synchronize on a locked object. All code that used to look for `LingeredApp.main` in the output now instead looks for `LingeredApp.steadyState`, which should always be there. I'm open to suggestions for a better name than "SteadyState" if you have one.
>> 
>> There are a few special cases with the tests that were modified that are described below:
>> 
>> Regarding the removal of `LingeredAppWithTrivialMain.java`, it was originally added to fix [JDK-8229118](https://bugs.openjdk.java.net/browse/JDK-8229118), which was an issue with the `ClhsdbFindPC` test failing on aarch64 when doing the `-Xcomp` run. It expected `LingeredApp.main()` to be compiled in that case, and for the PC of the frame to be in an `NMethod`. However, on aarch64 an uncommon-trap was causing it to be de-optimized, so the PC was in the interpreter instead, causing the test to fail. The fix was to instead rely on finding a trivial `LingeredAppWithTrivialMain.main()` frame in the stack trace since it would always be compiled. With the changes to instead have (and rely on) the `SteadyStateThread` and the presence of `LingeredApp.steadyState()`, the need for `LingeredAppWithTrivialMain` goes away. `LingeredApp.steadyState()` will always be compiled, even on aarch64.
>> 
>> Regarding `DebugdConnectTest.java`, it is listed in the CR as an affected test but no specified fix has been provided.
>>  None is needed. The issue was that it looked for `LingeredApp` in the jstack output, and it used to be the only place it would find it was in the main thread's stack trace, which sometimes could not be retrieved. Now it can also be found in the `SteadyStateThread`'s stack trace, which can always be retrieved.
>> 
>> Regarding `HeapDumpTest.java`, it used to check for `LingeredAppWithExtendedChars.main` and now it checks for `LingeredApp.steadyState`. It still is run with `LingeredAppWithExtendedChars`. The only thing special about that class is that it contains an extended unicode character used to reproduce [JDK-8028623](https://bugs.openjdk.java.net/browse/JDK-8028623), so it will still do that, even though it now checks for `LingeredApp.steadyState`.
>
> Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Use onSpinWait() instead of sleep(), and check for BLOCKED thread state.

Marked as reviewed by ysuenaga (Reviewer).

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

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