Skip to content

Commit

Permalink
Fix a destruction order issue in ThreadStatusUpdater
Browse files Browse the repository at this point in the history
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
  • Loading branch information
yhchiang committed Aug 15, 2016
1 parent deda159 commit b248e98
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions util/env_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit b248e98

Please sign in to comment.