Make //base:singleton thread-safe.

# `Singleton<T>`
* Access to both `once_` and `instance_` must be synchronized.
  * Note that `absl::once_flag` itself doesn't need any sync, as it is a sync primitive itself.
  * But we wrap it in a `std::optional`.
* The `instance_` could be put behind an atomic, but we need a mutex for `once_`.
* Once we have a mutex, we actually don't need `once_` because `instance_ == nullptr` inside critical section is the exact condition to perform initialization.
* Finalizer array must be put behind a mutex too.
* Logging utilities no longer depends on `//base:singleton`. Let's just use `LOG(FATAL)` on error.

# `SingletonMockable<T>`
* Access to `mock_` must be synchronized.
* We can simply use an atomic here.
* Use `absl::NoDestructor` for storing the real impl.

PiperOrigin-RevId: 728982963

src/base/BUILD.bazel

@@ -191,7 +191,12 @@ mozc_cc_library(
     srcs = ["singleton.cc"],
     hdrs = ["singleton.h"],
     visibility = ["//:__subpackages__"],
-    deps = ["@com_google_absl//absl/base"],
+    deps = [
+        "@com_google_absl//absl/base:core_headers",
+        "@com_google_absl//absl/base:no_destructor",
+        "@com_google_absl//absl/log",
+        "@com_google_absl//absl/synchronization",
+    ],
 )
 
 mozc_cc_test(

src/base/singleton.cc

@@ -29,59 +29,41 @@
 
 #include "base/singleton.h"
 
-#ifdef _WIN32
+#include <array>
-#include <windows.h>
+
-#else  // _WIN32
+#include "absl/base/attributes.h"
-#include <cstdlib>
+#include "absl/base/const_init.h"
-#endif  // _WIN32
+#include "absl/base/thread_annotations.h"
+#include "absl/log/log.h"
+#include "absl/synchronization/mutex.h"
 
 namespace mozc {
+namespace internal {
 namespace {
 
-constexpr size_t kMaxFinalizersSize = 256;
+ABSL_CONST_INIT absl::Mutex mu(absl::kConstInit);
-size_t g_finalizers_size = 0;
+ABSL_CONST_INIT std::array<void (*)(void), 256> finalizers
-
+    ABSL_GUARDED_BY(mu) = {};
-SingletonFinalizer::FinalizerFunc g_finalizers[kMaxFinalizersSize];
+ABSL_CONST_INIT int size ABSL_GUARDED_BY(mu) = 0;
-
-// We can't use CHECK logic for Singleton because CHECK (and LOG)
-// obtains the singleton LogStream by calling Singleton::get().  If
-// something goes wrong during Singleton::get of the LogStream, it
-// recursively calls Singleton::get again to report errors, which
-// leads an inifinite wait loop because the first Singleton::get locks
-// everything.
-// ExitWithError() exits the program without reporting errors, which
-// is not good but better than an inifinite loop.
-void ExitWithError() {
-  // This logic is copied from logging.h
-#ifdef _WIN32
-  ::RaiseException(::GetLastError(), EXCEPTION_NONCONTINUABLE, 0, nullptr);
-#else   // _WIN32
-  exit(-1);
-#endif  // _WIN32
-}
 
 }  // namespace
 
-void SingletonFinalizer::AddFinalizer(FinalizerFunc func) {
+void AddSingletonFinalizer(void (*finalizer)()) ABSL_LOCKS_EXCLUDED(mu) {
-  // When g_finalizers_size is equal to kMaxFinalizersSize,
+  absl::MutexLock lock(&mu);
-  // SingletonFinalizer::Finalize is called already.
+  if (size >= finalizers.size()) {
-  if (g_finalizers_size >= kMaxFinalizersSize) {
+    LOG(FATAL) << "Too many singletons";
-    ExitWithError();
   }
-  // This part is not thread safe.
+  finalizers[size++] = finalizer;
-  // When two different classes are instantiated at the same time,
-  // this code will raise an exception.
-  g_finalizers[g_finalizers_size++] = func;
 }
 
-void SingletonFinalizer::Finalize() {
+}  // namespace internal
-  // This part is not thread safe.
+
-  // When two different classes are instantiated at the same time,
+void FinalizeSingletons() ABSL_LOCKS_EXCLUDED(internal::mu) {
-  // this code will raise an exception.
+  absl::MutexLock lock(&internal::mu);
-  for (int i = static_cast<int>(g_finalizers_size) - 1; i >= 0; --i) {
+  for (auto func : internal::finalizers) {
-    (*g_finalizers[i])();
+    func();
   }
-  g_finalizers_size = 0;
+  internal::size = 0;
 }
 
 }  // namespace mozc

src/base/singleton.h

@@ -30,29 +30,32 @@
 #ifndef MOZC_BASE_SINGLETON_H_
 #define MOZC_BASE_SINGLETON_H_
 
-#include <optional>
+#include <atomic>
-#include <utility>
 
-#include "absl/base/call_once.h"
+#include "absl/base/attributes.h"
+#include "absl/base/const_init.h"
+#include "absl/base/no_destructor.h"
+#include "absl/base/thread_annotations.h"
+#include "absl/synchronization/mutex.h"
 
 namespace mozc {
+namespace internal {
 
-class SingletonFinalizer {
+// Do not call this method directly. Use `Singleton<T>` instead.
- public:
+void AddSingletonFinalizer(void (*finalizer)());
-  typedef void (*FinalizerFunc)();
 
-  // Do not call this method directly.
+}  // namespace internal
-  // use Singleton<Typename> instead.
-  static void AddFinalizer(FinalizerFunc func);
 
-  // Call Finalize() if you want to finalize
+// Destructs all singletons created by `Singleton<T>`. The primary usage is to
-  // all instances created by Sigleton.
+// call this right before unloading the Mozc UI for Windows DLL to avoid memory
-  //
+// leaks.
-  // Mozc UI for Windows (DLL) can call
+//
-  // SigletonFinalizer::Finalize()
+// Generally speaking, you SHOULD try to avoid singletons by injecting
-  // at an appropriate timing.
+// dependencies instead.
-  static void Finalize();
+//
-};
+// NOTE: This is a dangerous operation that can cause use-after-free when
+// misused.
+void FinalizeSingletons();
 
 // Thread-safe Singleton class.
 // Usage:
@@ -63,34 +66,36 @@ class SingletonFinalizer {
 template <typename T>
 class Singleton {
  public:
-  static T *get() {
+  static T *get() ABSL_LOCKS_EXCLUDED(mutex_) {
-    absl::call_once(*once_, &Singleton<T>::Init);
+    {
+      absl::ReaderMutexLock lock(&mutex_);  // NOLINT: In the program's steady
+                                            // state there's no write lock.
+      if (instance_ != nullptr) {
+        return instance_;
+      }
+    }
+
+    absl::MutexLock lock(&mutex_);
+    if (instance_ == nullptr) {
+      instance_ = new T();
+      internal::AddSingletonFinalizer(&Singleton<T>::Delete);
+    }
     return instance_;
   }
 
   // TEST ONLY! Do not call this method in production code.
-  static void Delete() {
+  static void Delete() ABSL_LOCKS_EXCLUDED(mutex_) {
+    absl::MutexLock lock(&mutex_);
     delete instance_;
     instance_ = nullptr;
-    once_.emplace();  // Reconstruct absl::once_flag in place.
   }
 
  private:
-  static void Init() {
+  ABSL_CONST_INIT static inline absl::Mutex mutex_ =
-    SingletonFinalizer::AddFinalizer(&Singleton<T>::Delete);
+      absl::Mutex(absl::kConstInit);
-    instance_ = new T;
+  ABSL_CONST_INIT static inline T *instance_ ABSL_GUARDED_BY(mutex_) = nullptr;
-  }
-
-  static std::optional<absl::once_flag> once_;
-  static T *instance_;
 };
 
-template <typename T>
-std::optional<absl::once_flag> Singleton<T>::once_(std::in_place);
-
-template <typename T>
-T *Singleton<T>::instance_ = nullptr;
-
 // SingletonMockable class.
 // Usage: (quote from clock.cc)
 //
@@ -107,21 +112,21 @@ template <class Interface, class Impl>
 class SingletonMockable {
  public:
   static Interface *Get() {
-    if (mock_) {
+    if (Interface *mock = mock_.load(std::memory_order_acquire)) {
-      return mock_;
+      return mock;
     }
-    static Impl *impl = new Impl();
+    static absl::NoDestructor<Impl> impl;
-    return impl;
+    return impl.get();
+  }
+
+  static void SetMock(Interface *mock) {
+    mock_.store(mock, std::memory_order_release);
   }
-  static void SetMock(Interface *mock) { mock_ = mock; }
 
  private:
-  static Interface *mock_;
+  ABSL_CONST_INIT static inline std::atomic<Interface *> mock_ = nullptr;
 };
 
-template <class Interface, class Impl>
-Interface *SingletonMockable<Interface, Impl>::mock_ = nullptr;
-
 }  // namespace mozc
 
 #endif  // MOZC_BASE_SINGLETON_H_

src/server/mozc_server.cc

@@ -134,7 +134,7 @@ int MozcServer::Run() {
 }
 
 int MozcServer::Finalize() {
-  mozc::SingletonFinalizer::Finalize();
+  mozc::FinalizeSingletons();
   return 0;
 }
 

src/win32/tip/tip_dll_module.cc

@@ -66,7 +66,7 @@ void TipDllModule::PrepareForShutdown() {
   // Windows 8. Thus we must not shut down libraries that cannot be designed to
   // be re-initializable. For instance, we must not call following functions
   // here.
-  // - SingletonFinalizer::Finalize()            - b/10233768
+  // - mozc::FinalizeSingletons()                - b/10233768
   // - mozc::protobuf::ShutdownProtobufLibrary() - b/2126375
   absl::call_once(uninitialize_once_, &TipShutdownCrashReportHandler);
 }