Fix a destruction order issue in ThreadStatusUpdater

Summary:
Env holds a pointer of ThreadStatusUpdater, which will be deleted when
Env is deleted.  However, in case a rocksdb database is deleted after
Env is deleted.  Then this will introduce a free-after-use of this
ThreadStatusUpdater.

This patch fix this by never deleting the ThreadStatusUpdater in Env,
which is in general safe as Env is a singleton in most cases.

Test Plan: thread_list_test

Reviewers: andrewkr, sdong

Reviewed By: sdong

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D59187

diff --git a/util/env_posix.cc b/util/env_posix.cc
index 0c14c03fee096102ebe445a23901b4845a5b0454..6e1141d7a0df470f4f09127eb5a19db6f263935f 100644 (file)
--- a/util/env_posix.cc
+++ b/util/env_posix.cc
@@ -129,9 +129,13 @@ class PosixEnv : public Env {
     for (int pool_id = 0; pool_id < Env::Priority::TOTAL; ++pool_id) {
       thread_pools_[pool_id].JoinAllThreads();
     }
-    // All threads must be joined before the deletion of
-    // thread_status_updater_.
-    delete thread_status_updater_;
+    // Delete the thread_status_updater_ only when the current Env is not
+    // Env::Default().  This is to avoid the free-after-use error when
+    // Env::Default() is destructed while some other child threads are
+    // still trying to update thread status.
+    if (this != Env::Default()) {
+      delete thread_status_updater_;
+    }
   }
 
   void SetFD_CLOEXEC(int fd, const EnvOptions* options) {