Revert back to the basic bugfix and remove AsyncLogGtest.java from ProblemList.

Author: Xin Liu

src/hotspot/share/logging/logAsyncWriter.cpp

@@ -102,7 +102,7 @@ class AsyncLogMapIterator {
     using none = LogTagSetMapping<LogTag::__NO_TAG>;
 
     if (*counter > 0) {
-      LogDecorations decorations(LogLevel::Warning, none::tagset(), LogDecorators::All);
+      LogDecorations decorations(LogLevel::Warning, none::tagset(), output->decorators());
       stringStream ss;
       ss.print(UINT32_FORMAT_W(6) " messages dropped due to async logging", *counter);
       AsyncLogMessage msg(*output, decorations, ss.as_string(true /*c_heap*/));

src/hotspot/share/logging/logConfiguration.cpp

@@ -253,41 +253,19 @@ void LogConfiguration::configure_output(size_t idx, const LogSelectionList& sele
     on_level[level]++;
   }
 
-  // MT-Safety
-  // ConfigurationLock can only guarantee that only one thread is executing reconfiguration. This function still must
-  // be MT-safe because logsites in other threads may be executing in parallel.
-  //
-  // Reconfiguration: Unified logging allows users to dynamically change tags and decorators of a log output.
-  //
-  // A synchronization 'wait_until_no_readers()' is imposed inside of 'ts->set_output_level(output, level)' above
-  // if setting has changed. It guarantees that all logs either synchronous writing or enqueuing to the async buffer
-  // see the new tags and decorators. It's worth noting that the synchronization happens even level doesn't change.
-  //
-  // LogDecorator is a set of decorators represented in a uint. sizeof(uint) is not greater than a machine word,
-  // so store of it is atomic on the mainstream processors. I.e. readers see either its older value or new value.
-  // ts->update_decorators(decorators) above is a union operation of the existing decorators at different levels.
-  // It's safe to do output->set_decorators(decorators) because decorators is a subset of relevant Tagsets' decorators.
-  // After updating output's decorators, it's still safe to shrink all decorators of tagsets.
-  //
-  // There are 2 hazards in async logging. A flush operation guarantees to all pending messages in buffer are written
-  // before returning. Therefore, the hardards won't appear. It's a nop if async logging is not set.
-  // 1. asynclog buffer may be holding some log messages with previous decorators.
-  // 2. asynclog buffer may be holding some log messages targeting to the output 'idx'. It has been disabled by new setting,
-  // eg. all=off and is about to be purged in delete_output(idx).
-  //
-  AsyncLogWriter::flush();
-
   // It is now safe to set the new decorators for the actual output
   output->set_decorators(decorators);
 
-  OrderAccess::storestore();
-
   // Update the decorators on all tagsets to get rid of unused decorators
   for (LogTagSet* ts = LogTagSet::first(); ts != NULL; ts = ts->next()) {
     ts->update_decorators();
   }
 
   if (!enabled && idx > 1) {
+    // User may disable a logOuput like this:
+    // LogConfiguration::parse_log_arguments(filename, "all=off", "", "", &stream);
+    // Just be conservative. Flush them all before deleting idx.
+    AsyncLogWriter::flush();
     // Output is unused and should be removed, unless it is stdout/stderr (idx < 2)
     delete_output(idx);
     return;

src/hotspot/share/logging/logDecorators.cpp

@@ -25,19 +25,7 @@
 #include "logging/logDecorators.hpp"
 #include "runtime/os.hpp"
 
-template <LogDecorators::Decorator d>
-struct AllBitmask {
-  // Use recursive template deduction to calculate the bitmask of all decorations.
-  static const uint _value = (1 << d) | AllBitmask<static_cast<LogDecorators::Decorator>(d + 1)>::_value;
-};
-
-template<>
-struct AllBitmask<LogDecorators::Count> {
-  static const uint _value = 0;
-};
-
 const LogDecorators LogDecorators::None = LogDecorators(0);
-const LogDecorators LogDecorators::All  = LogDecorators(AllBitmask<time_decorator>::_value);
 
 const char* LogDecorators::_name[][2] = {
 #define DECORATOR(n, a) {#n, #a},

src/hotspot/share/logging/logDecorators.hpp

@@ -82,7 +82,6 @@ class LogDecorators {
 
  public:
   static const LogDecorators None;
-  static const LogDecorators All;
 
   LogDecorators() : _decorators(DefaultDecoratorsMask) {
   }

src/hotspot/share/logging/logFileOutput.cpp

@@ -33,19 +33,6 @@
 #include "utilities/globalDefinitions.hpp"
 #include "utilities/defaultStream.hpp"
 
-class RotationLocker : public StackObj {
-  Semaphore& _sem;
-
- public:
-  RotationLocker(Semaphore& sem) : _sem(sem) {
-    sem.wait();
-  }
-
-  ~RotationLocker() {
-    _sem.signal();
-  }
-};
-
 const char* const LogFileOutput::Prefix = "file=";
 const char* const LogFileOutput::FileOpenMode = "a";
 const char* const LogFileOutput::PidFilenamePlaceholder = "%p";
@@ -299,12 +286,7 @@ bool LogFileOutput::initialize(const char* options, outputStream* errstream) {
 }
 
 int LogFileOutput::write_blocking(const LogDecorations& decorations, const char* msg) {
-  RotationLocker lock(_rotation_semaphore);
-  if (_stream == NULL) {
-    // An error has occurred with this output, avoid writing to it.
-    return 0;
-  }
-
+  _rotation_semaphore.wait();
   int written = LogFileStreamOutput::write(decorations, msg);
   if (written > 0) {
     _current_size += written;
@@ -313,6 +295,7 @@ int LogFileOutput::write_blocking(const LogDecorations& decorations, const char*
       rotate();
     }
   }
+  _rotation_semaphore.signal();
 
   return written;
 }
@@ -344,7 +327,7 @@ int LogFileOutput::write(LogMessageBuffer::Iterator msg_iterator) {
     return 0;
   }
 
-  RotationLocker lock(_rotation_semaphore);
+  _rotation_semaphore.wait();
   int written = LogFileStreamOutput::write(msg_iterator);
   if (written > 0) {
     _current_size += written;
@@ -353,6 +336,7 @@ int LogFileOutput::write(LogMessageBuffer::Iterator msg_iterator) {
       rotate();
     }
   }
+  _rotation_semaphore.signal();
 
   return written;
 }
@@ -379,11 +363,13 @@ void LogFileOutput::force_rotate() {
     // Rotation not possible
     return;
   }
-  RotationLocker lock(_rotation_semaphore);
+  _rotation_semaphore.wait();
   rotate();
+  _rotation_semaphore.signal();
 }
 
 void LogFileOutput::rotate() {
+
   if (fclose(_stream)) {
     jio_fprintf(defaultStream::error_stream(), "Error closing file '%s' during log rotation (%s).\n",
                 _file_name, os::strerror(errno));

src/hotspot/share/logging/logTagSet.cpp

@@ -73,19 +73,15 @@ bool LogTagSet::has_output(const LogOutput* output) {
 }
 
 void LogTagSet::log(LogLevelType level, const char* msg) {
-  LogOutputList::Iterator it = _output_list.iterator(level);
   LogDecorations decorations(level, *this, _decorators);
-
-  for (; it != _output_list.end(); it++) {
+  for (LogOutputList::Iterator it = _output_list.iterator(level); it != _output_list.end(); it++) {
     (*it)->write(decorations, msg);
   }
 }
 
 void LogTagSet::log(const LogMessageBuffer& msg) {
-  LogOutputList::Iterator it = _output_list.iterator(msg.least_detailed_level());
   LogDecorations decorations(LogLevel::Invalid, *this, _decorators);
-
-  for (; it != _output_list.end(); it++) {
+  for (LogOutputList::Iterator it = _output_list.iterator(msg.least_detailed_level()); it != _output_list.end(); it++) {
     (*it)->write(msg.iterator(it.level(), decorations));
   }
 }

test/hotspot/gtest/logging/logTestFixture.cpp

@@ -46,6 +46,7 @@ LogTestFixture::LogTestFixture() : _n_snapshots(0), _configuration_snapshot(NULL
 }
 
 LogTestFixture::~LogTestFixture() {
+  AsyncLogWriter::flush();
   restore_config();
   clear_snapshot();
   delete_file(TestLogFileName);

test/hotspot/gtest/logging/test_logConfiguration.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -23,7 +23,6 @@
 
 #include "precompiled.hpp"
 #include "jvm.h"
-#include "concurrentTestRunner.inline.hpp"
 #include "logTestFixture.hpp"
 #include "logTestUtils.inline.hpp"
 #include "logging/logConfiguration.hpp"
@@ -229,91 +228,6 @@ TEST_VM_F(LogConfigurationTest, reconfigure_decorators) {
   EXPECT_TRUE(is_described("#1: stderr all=off none (reconfigured)\n")) << "Expecting no decorators";
 }
 
-class ConcurrentLogsite : public TestRunnable {
-  int _id;
-
- public:
-  ConcurrentLogsite(int id) : _id(id) {}
-  void runUnitTest() const override {
-    log_debug(logging)("ConcurrentLogsite %d emits a log", _id);
-  }
-};
-
-// Dynamically change decorators while loggings are emitting.
-TEST_VM_F(LogConfigurationTest, reconfigure_decorators_MT) {
-  const int nrOfThreads = 2;
-  ConcurrentLogsite logsites[nrOfThreads] = {0, 1};
-  Semaphore done(0);
-  const long testDurationMillis = 1000;
-  UnitTestThread* t[nrOfThreads];
-
-  set_log_config(TestLogFileName, "logging=debug", "none", "filecount=0");
-  set_log_config("stdout", "all=off", "none");
-  set_log_config("stderr", "all=off", "none");
-  for (int i = 0; i < nrOfThreads; ++i) {
-    t[i] = new UnitTestThread(&logsites[i], &done, testDurationMillis);
-  }
-
-  for (int i = 0; i < nrOfThreads; i++) {
-    t[i]->doit();
-  }
-
-  jlong time_start = os::elapsed_counter();
-  while (true) {
-    jlong elapsed = (jlong)TimeHelper::counter_to_millis(os::elapsed_counter() - time_start);
-    if (elapsed > testDurationMillis) {
-      break;
-    }
-
-    // Take turn logging with different decorators, either None or All.
-    set_log_config(TestLogFileName, "logging=debug", "none");
-    set_log_config(TestLogFileName, "logging=debug", _all_decorators);
-  }
-
-  for (int i = 0; i < nrOfThreads; ++i) {
-    done.wait();
-  }
-}
-
-// Dynamically change tags while loggings are emitting.
-TEST_VM_F(LogConfigurationTest, reconfigure_tags_MT) {
-  const int nrOfThreads = 4;
-  ConcurrentLogsite logsites[nrOfThreads] = {0, 1, 2, 3};
-  Semaphore done(0);
-  const long testDurationMillis = 1000;
-  UnitTestThread* t[nrOfThreads];
-
-  set_log_config(TestLogFileName, "logging=debug", "", "filecount=0");
-  set_log_config("stdout", "all=off", "none");
-  set_log_config("stderr", "all=off", "none");
-
-  for (int i = 0; i < nrOfThreads; ++i) {
-    t[i] = new UnitTestThread(&logsites[i], &done, testDurationMillis);
-  }
-
-  for (int i = 0; i < nrOfThreads; i++) {
-    t[i]->doit();
-  }
-
-  jlong time_start = os::elapsed_counter();
-  while (true) {
-    jlong elapsed = (jlong)TimeHelper::counter_to_millis(os::elapsed_counter() - time_start);
-    if (elapsed > testDurationMillis) {
-      break;
-    }
-
-    // turn on/off the tagset 'logging'.
-    set_log_config(TestLogFileName, "logging=off");
-    set_log_config(TestLogFileName, "logging=debug", "", "filecount=0");
-    // sleep a prime number milliseconds to allow concurrent logsites write logs
-    os::naked_short_nanosleep(137);
-  }
-
-  for (int i = 0; i < nrOfThreads; ++i) {
-    done.wait();
-  }
-}
-
 // Test that invalid options cause configuration errors
 TEST_VM_F(LogConfigurationTest, invalid_configure_options) {
   LogConfiguration::disable_logging();

test/hotspot/gtest/logging/test_logDecorators.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -210,13 +210,6 @@ TEST(LogDecorators, none) {
   }
 }
 
-TEST(LogDecorators, all) {
-  LogDecorators dec = LogDecorators::All;
-  for (size_t i = 0; i < LogDecorators::Count; i++) {
-    EXPECT_TRUE(dec.is_decorator(decorator_array[i]));
-  }
-}
-
 TEST(LogDecorators, is_empty) {
   LogDecorators def, none = LogDecorators::None;
   EXPECT_FALSE(def.is_empty());