Hi Chris,

On 4/03/2021 6:09 am, Chris Plummer 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.

Doesn't jstack force all threads to be walkable via a safepoint or handshake?

David
-----

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`.

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

Commit messages:
  - Use full name of SteadStateThread
  - Create and use SteadyStateThread stack for jstack queries.

Changes: https://git.openjdk.java.net/jdk/pull/2700/files
  Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2700&range=00
   Issue: https://bugs.openjdk.java.net/browse/JDK-8243455
   Stats: 119 lines in 11 files changed: 43 ins; 36 del; 40 mod
   Patch: https://git.openjdk.java.net/jdk/pull/2700.diff
   Fetch: git fetch https://git.openjdk.java.net/jdk pull/2700/head:pull/2700

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

Reply via email to