On Tue, 23 Feb 2021 23:34:31 GMT, Chris Plummer <cjplum...@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`. test/lib/jdk/test/lib/apps/LingeredApp.java line 531: > 529: // although this probably is not necessary to guarantee that > the > 530: // stack trace is readable. > 531: Thread.sleep(100); Can we wait to change the state of `steadyStateThread` to `BLOCKED`? It is more robustness than `Thread.sleep(100)`. ------------- PR: https://git.openjdk.java.net/jdk/pull/2700
