On Wed, 3 Mar 2021 20:13:56 GMT, Leonid Mesnik <lmes...@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`. > > Marked as reviewed by lmesnik (Committer). > Doesn't jstack force all threads to be walkable via a safepoint or > handshake? Nope. If it did it couldn't debug core files or a hung process. That's why SA has so much code to safe guard against walking off the deep end when doing things like stack walking and heap scanning. BTW, you might be thinking of the Attach API version of `jstack`, which does safepoint. These tests all involve the SA version of `jstack`. ------------- PR: https://git.openjdk.java.net/jdk/pull/2700
