From b248e98cf990cb72663c350c8f00704a21bc60db Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang Date: Mon, 15 Aug 2016 09:04:55 -0700 Subject: [PATCH] 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 --- util/env_posix.cc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/util/env_posix.cc b/util/env_posix.cc index 0c14c03fe..6e1141d7a 100644 --- 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) {