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

Reviewed-by: aboldtch, dholmes

Author: jdksjolen

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;
 }

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);
 

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) \

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);
   }

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")                   \

test/hotspot/jtreg/runtime/logging/AsyncDeathTest.java

@@ -38,17 +38,24 @@
 
 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");
     }
 }