RFR: 8350214: Test gtest/AsyncLogGtest.java fails after JDK-8349755

[Axel Boldt-Christmas]

On Wed, 19 Feb 2025 14:44:51 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> Hi,
>> 
>> [8349755](https://bugs.openjdk.org/browse/JDK-8349755) introduced a bug in debug builds when running with -Xlog:async -Xlog:all=debug. Specifically, this will cause UL to select for deathtest and deathtest2, which should only be selected when explicitly asked for.
>> 
>> In order to fix this, I've added some develop-only globals. We now only log if either of the testing globals are set, and we only crash on recursive logging if `TestingAsyncLoggingDeathTestNoCrash` is set to `false`.
>> 
>> Let's go through the cases:
>> 
>> 
>> -Xlog:all=debug => deathtest isn't logged because the globals aren't set => Only non-deathtest recursive logs crash (as it should be)
>> -Xlog:all=debug + TestingAsyncLoggingDeathTestNoCrash => deathtest is logged, but TestingAsyncLoggingDeathTestNoCrash is true so the ShouldNotReachHere branch isn't taken
>> -Xlog:all=debug + TestingAsyncLoggingDeathTest => deathtest is logged, and it crashes
>
> src/hotspot/share/runtime/globals.hpp line 487:
> 
>> 485:           "Recursive logging death test")                                   \
>> 486:   develop(bool, TestingAsyncLoggingDeathTestNoCrash, falseInDebug,          \
>> 487:           "Recursive logging death test (no crash)")                        \
> 
> `falseInDebug` means `true` when `ASSERT` is not defined. Think these should just be `false`.

Even if it does not matter because we guard the use of the flags by `ASSERT`, it looks strange that the values end up being `true`/

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23695#discussion_r1961831467