From 10bf48a6b0b796b48cdca15250e1ee7e7be83c82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Sj=C3=B6len?= Date: Thu, 20 Feb 2025 15:50:24 +0000 Subject: [PATCH] 8350214: Test gtest/AsyncLogGtest.java fails after JDK-8349755 Reviewed-by: aboldtch, dholmes --- src/hotspot/share/logging/logAsyncWriter.cpp | 14 ++++++++++---- src/hotspot/share/logging/logAsyncWriter.hpp | 1 - src/hotspot/share/logging/logTag.hpp | 1 - src/hotspot/share/logging/logTagSet.cpp | 8 +------- src/hotspot/share/runtime/globals.hpp | 4 ++++ .../jtreg/runtime/logging/AsyncDeathTest.java | 15 +++++++++++---- 6 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/hotspot/share/logging/logAsyncWriter.cpp b/src/hotspot/share/logging/logAsyncWriter.cpp index 44758958dd3..737f001c049 100644 --- a/src/hotspot/share/logging/logAsyncWriter.cpp +++ b/src/hotspot/share/logging/logAsyncWriter.cpp @@ -30,8 +30,8 @@ #include "memory/resourceArea.hpp" #include "runtime/atomic.hpp" #include "runtime/os.inline.hpp" +#include "runtime/globals.hpp" -DEBUG_ONLY(bool AsyncLogWriter::ignore_recursive_logging = false;) class AsyncLogWriter::AsyncLogLocker : public StackObj { static Thread* _holder; @@ -116,10 +116,11 @@ bool AsyncLogWriter::is_enqueue_allowed() { // Do not log while holding the Async log lock. // Try to catch possible occurrences in debug builds. #ifdef ASSERT - if (!AsyncLogWriter::ignore_recursive_logging) { + if (!TestingAsyncLoggingDeathTestNoCrash) { ShouldNotReachHere(); } #endif // ASSERT + return false; } @@ -142,8 +143,13 @@ bool AsyncLogWriter::enqueue(LogFileStreamOutput& output, const LogDecorations& } AsyncLogLocker locker; - DEBUG_ONLY(log_debug(deathtest)("Induce a recursive log for testing (for crashing)");) - DEBUG_ONLY(log_debug(deathtest2)("Induce a recursive log for testing");) + +#ifdef ASSERT + if (TestingAsyncLoggingDeathTest || TestingAsyncLoggingDeathTestNoCrash) { + log_debug(deathtest)("Induce a recursive log for testing"); + } +#endif // ASSERT + AsyncLogWriter::instance()->enqueue_locked(&output, decorations, msg); return true; } diff --git a/src/hotspot/share/logging/logAsyncWriter.hpp b/src/hotspot/share/logging/logAsyncWriter.hpp index e2c8cb36e07..00818634bdd 100644 --- a/src/hotspot/share/logging/logAsyncWriter.hpp +++ b/src/hotspot/share/logging/logAsyncWriter.hpp @@ -198,7 +198,6 @@ class AsyncLogWriter : public NonJavaThread { static bool is_enqueue_allowed(); public: - DEBUG_ONLY(static bool ignore_recursive_logging;) static bool enqueue(LogFileStreamOutput& output, const LogDecorations& decorations, const char* msg); static bool enqueue(LogFileStreamOutput& output, LogMessageBuffer::Iterator msg_iterator); diff --git a/src/hotspot/share/logging/logTag.hpp b/src/hotspot/share/logging/logTag.hpp index c3db84930ad..d61e2461e8d 100644 --- a/src/hotspot/share/logging/logTag.hpp +++ b/src/hotspot/share/logging/logTag.hpp @@ -69,7 +69,6 @@ class outputStream; LOG_TAG(datacreation) \ LOG_TAG(dcmd) \ DEBUG_ONLY(LOG_TAG(deathtest)) /* Log Internal death test tag */ \ - DEBUG_ONLY(LOG_TAG(deathtest2)) /* Log Internal death test tag */ \ LOG_TAG(decoder) \ LOG_TAG(defaultmethods) \ LOG_TAG(deoptimization) \ diff --git a/src/hotspot/share/logging/logTagSet.cpp b/src/hotspot/share/logging/logTagSet.cpp index 718627be3a5..f7b0e01331c 100644 --- a/src/hotspot/share/logging/logTagSet.cpp +++ b/src/hotspot/share/logging/logTagSet.cpp @@ -78,13 +78,7 @@ void LogTagSet::log(LogLevelType level, const char* msg) { // the implied memory order of Atomic::add(). LogOutputList::Iterator it = _output_list.iterator(level); LogDecorations decorations(level, *this, _decorators); -#ifdef ASSERT - // If we log for tag deathtest2 then we're testing that recursive logging works. - // In this case, do not crash when detecting recursive logging. - if (this->contains(LogTagType::_deathtest2)) { - AsyncLogWriter::ignore_recursive_logging = true; - } -#endif + for (; it != _output_list.end(); it++) { (*it)->write(decorations, msg); } diff --git a/src/hotspot/share/runtime/globals.hpp b/src/hotspot/share/runtime/globals.hpp index 11c13fe3c9e..d3bdb0c68b2 100644 --- a/src/hotspot/share/runtime/globals.hpp +++ b/src/hotspot/share/runtime/globals.hpp @@ -481,6 +481,10 @@ const int ObjectAlignmentInBytes = 8; \ develop(bool, ZapTLAB, trueInDebug, \ "Zap allocated TLABs") \ + develop(bool, TestingAsyncLoggingDeathTest, false, \ + "Recursive logging death test") \ + develop(bool, TestingAsyncLoggingDeathTestNoCrash, false, \ + "Recursive logging death test (no crash)") \ \ product(bool, ExecutingUnitTests, false, \ "Whether the JVM is running unit tests or not") \ diff --git a/test/hotspot/jtreg/runtime/logging/AsyncDeathTest.java b/test/hotspot/jtreg/runtime/logging/AsyncDeathTest.java index 2dff887f108..cd4a50f65c1 100644 --- a/test/hotspot/jtreg/runtime/logging/AsyncDeathTest.java +++ b/test/hotspot/jtreg/runtime/logging/AsyncDeathTest.java @@ -38,17 +38,24 @@ import jdk.test.lib.process.OutputAnalyzer; public class AsyncDeathTest { public static void main(String[] args) throws Exception { - // For deathtest we expect the VM to reach ShouldNotReachHere() and die + // TestingAsyncLoggingDeathTest is set: We expect the VM to reach ShouldNotReachHere() and die. ProcessBuilder pb = - ProcessTools.createLimitedTestJavaProcessBuilder("-Xlog:async", "-Xlog:os,deathtest=debug", "-XX:-CreateCoredumpOnCrash", "--version"); + ProcessTools.createLimitedTestJavaProcessBuilder("-Xlog:async", "-Xlog:os,deathtest=debug", "-XX:-CreateCoredumpOnCrash", "-XX:+TestingAsyncLoggingDeathTest", "--version"); OutputAnalyzer output = new OutputAnalyzer(pb.start()); output.shouldNotHaveExitValue(0); output.shouldNotContain("Induce a recursive log for testing"); - // For deathtest2 we expect the VM to ignore that recursive logging has been detected and is handled by printing synchronously. + // TestingAsyncLoggingDeathTestNoCrash is set: We expect the VM to ignore that recursive logging has been detected and the logging should be handled by printing synchronously. ProcessBuilder pb2 = - ProcessTools.createLimitedTestJavaProcessBuilder("-Xlog:async", "-Xlog:os,deathtest2=debug", "--version"); + ProcessTools.createLimitedTestJavaProcessBuilder("-Xlog:async", "-Xlog:os,deathtest=debug", "-XX:+TestingAsyncLoggingDeathTestNoCrash" , "--version"); OutputAnalyzer output2 = new OutputAnalyzer(pb2.start()); output2.shouldHaveExitValue(0); output2.shouldContain("Induce a recursive log for testing"); + + // For -Xlog:all=debug but without any global set, the test should succeed and not contain the recursive message. + ProcessBuilder pb3 = + ProcessTools.createLimitedTestJavaProcessBuilder("-Xlog:async", "-Xlog:all=debug", "--version"); + OutputAnalyzer output3 = new OutputAnalyzer(pb3.start()); + output3.shouldHaveExitValue(0); + output3.shouldNotContain("Induce a recursive log for testing"); } }