From c3ef8581d2b69d3b5e0ddd637da8b67c6adc043a Mon Sep 17 00:00:00 2001
From: Kai Yang <kfstorm@outlook.com>
Date: Sun, 15 Dec 2019 17:52:34 +0800
Subject: [PATCH] [Java] fix UT segmentation fault on exit (#6455)

* fix segmentation fault in Java test

* update comments

* address comments
---
 .../lib/java/org_ray_runtime_RayNativeRuntime.cc         | 5 ++++-
 src/ray/util/logging.cc                                  | 9 +++++++++
 src/ray/util/logging.h                                   | 3 +++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/ray/core_worker/lib/java/org_ray_runtime_RayNativeRuntime.cc b/src/ray/core_worker/lib/java/org_ray_runtime_RayNativeRuntime.cc
index 901c84660..7ba94f67a 100644
--- a/src/ray/core_worker/lib/java/org_ray_runtime_RayNativeRuntime.cc
+++ b/src/ray/core_worker/lib/java/org_ray_runtime_RayNativeRuntime.cc
@@ -108,7 +108,10 @@ JNIEXPORT void JNICALL Java_org_ray_runtime_RayNativeRuntime_nativeSetup(JNIEnv
                                                                          jstring logDir) {
   std::string log_dir = JavaStringToNativeString(env, logDir);
   ray::RayLog::StartRayLog("java_worker", ray::RayLogLevel::INFO, log_dir);
-  // TODO (kfstorm): If we add InstallFailureSignalHandler here, Java test may crash.
+  // TODO (kfstorm): We can't InstallFailureSignalHandler here, because JVM already
+  // installed its own signal handler. It's possible to fix this by chaining signal
+  // handlers. But it's not easy. See
+  // https://docs.oracle.com/javase/9/troubleshoot/handle-signals-and-exceptions.htm.
 }
 
 JNIEXPORT void JNICALL Java_org_ray_runtime_RayNativeRuntime_nativeShutdownHook(JNIEnv *,
diff --git a/src/ray/util/logging.cc b/src/ray/util/logging.cc
index 27425b342..b1e72eeb1 100644
--- a/src/ray/util/logging.cc
+++ b/src/ray/util/logging.cc
@@ -85,6 +85,7 @@ typedef ray::CerrLog LoggingProvider;
 RayLogLevel RayLog::severity_threshold_ = RayLogLevel::INFO;
 std::string RayLog::app_name_ = "";
 std::string RayLog::log_dir_ = "";
+bool RayLog::is_failure_signal_handler_installed_ = false;
 
 #ifdef RAY_USE_GLOG
 using namespace google;
@@ -167,6 +168,9 @@ void RayLog::StartRayLog(const std::string &app_name, RayLogLevel severity_thres
 
 void RayLog::UninstallSignalAction() {
 #ifdef RAY_USE_GLOG
+  if (!is_failure_signal_handler_installed_) {
+    return;
+  }
   RAY_LOG(DEBUG) << "Uninstall signal handlers.";
   // This signal list comes from glog's signalhandler.cc.
   // https://github.com/google/glog/blob/master/src/signalhandler.cc#L58-L70
@@ -184,6 +188,7 @@ void RayLog::UninstallSignalAction() {
     RAY_CHECK(sigaction(signal_num, &sig_action, NULL) == 0);
   }
 #endif
+  is_failure_signal_handler_installed_ = false;
 #endif
 }
 
@@ -198,7 +203,11 @@ void RayLog::ShutDownRayLog() {
 
 void RayLog::InstallFailureSignalHandler() {
 #ifdef RAY_USE_GLOG
+  if (is_failure_signal_handler_installed_) {
+    return;
+  }
   google::InstallFailureSignalHandler();
+  is_failure_signal_handler_installed_ = true;
 #endif
 }
 
diff --git a/src/ray/util/logging.h b/src/ray/util/logging.h
index da5cd5f25..4539eea78 100644
--- a/src/ray/util/logging.h
+++ b/src/ray/util/logging.h
@@ -128,6 +128,9 @@ class RayLog : public RayLogBase {
   /// The directory where the log files are stored.
   /// If this is empty, logs are printed to stdout.
   static std::string log_dir_;
+  /// This flag is used to avoid calling UninstallSignalAction in ShutDownRayLog if
+  /// InstallFailureSignalHandler was not called.
+  static bool is_failure_signal_handler_installed_;
 
  protected:
   virtual std::ostream &Stream();