Commit 7d707de4 authored by Misha Shneerson's avatar Misha Shneerson Committed by Facebook GitHub Bot

Capture correct TID passed to deadlock detector

Summary:
we are currently capture the ID of the thread that is spawning IO threads, not
the ids of the IO threads themselves. Fix it.

Reviewed By: yfeldblum

Differential Revision: D31577084

fbshipit-source-id: 869f9ad20ad379327db513501392bd6bc1ced7e8
parent f704135e
...@@ -34,10 +34,12 @@ void IOThreadPoolDeadlockDetectorObserver::threadStarted( ...@@ -34,10 +34,12 @@ void IOThreadPoolDeadlockDetectorObserver::threadStarted(
auto eventBase = folly::IOThreadPoolExecutor::getEventBase(h); auto eventBase = folly::IOThreadPoolExecutor::getEventBase(h);
// This Observer only works with IOThreadPoolExecutor class. // This Observer only works with IOThreadPoolExecutor class.
CHECK_NOTNULL(eventBase); CHECK_NOTNULL(eventBase);
auto thid = folly::to<std::string>(folly::getOSThreadID()); eventBase->runInEventBaseThread([=] {
auto name = name_ + ":" + thid; auto thid = folly::to<std::string>(folly::getOSThreadID());
detectors_.wlock()->insert_or_assign( auto name = name_ + ":" + thid;
h, deadlockDetectorFactory_->create(eventBase, name)); detectors_.wlock()->insert_or_assign(
h, deadlockDetectorFactory_->create(eventBase, name));
});
} }
void IOThreadPoolDeadlockDetectorObserver::threadStopped( void IOThreadPoolDeadlockDetectorObserver::threadStopped(
......
...@@ -45,14 +45,21 @@ class DeadlockDetectorFactoryMock : public DeadlockDetectorFactory { ...@@ -45,14 +45,21 @@ class DeadlockDetectorFactoryMock : public DeadlockDetectorFactory {
std::string expectedName = std::string expectedName =
fmt::format("TestPool:{}", folly::getOSThreadID()); fmt::format("TestPool:{}", folly::getOSThreadID());
EXPECT_EQ(expectedName, name); EXPECT_EQ(expectedName, name);
name_ = name;
baton.post();
return std::make_unique<DeadlockDetectorMock>(counter_); return std::make_unique<DeadlockDetectorMock>(counter_);
} }
int32_t getCounter() { return counter_->load(); } int32_t getCounter() { return counter_->load(); }
std::string getName() const { return name_.copy(); }
folly::Baton<> baton;
private: private:
std::shared_ptr<std::atomic<int32_t>> counter_ = std::shared_ptr<std::atomic<int32_t>> counter_ =
std::make_shared<std::atomic<int32_t>>(0); std::make_shared<std::atomic<int32_t>>(0);
folly::Synchronized<std::string> name_;
}; };
TEST( TEST(
...@@ -63,6 +70,9 @@ TEST( ...@@ -63,6 +70,9 @@ TEST(
auto executor = std::make_shared<IOThreadPoolExecutor>( auto executor = std::make_shared<IOThreadPoolExecutor>(
1, std::make_shared<folly::NamedThreadFactory>("TestPool")); 1, std::make_shared<folly::NamedThreadFactory>("TestPool"));
pid_t thid;
executor->getEventBase()->runInEventBaseThreadAndWait(
[&] { thid = folly::getOSThreadID(); });
auto observer = std::make_shared<folly::IOThreadPoolDeadlockDetectorObserver>( auto observer = std::make_shared<folly::IOThreadPoolDeadlockDetectorObserver>(
deadlockDetectorFactory.get(), "TestPool"); deadlockDetectorFactory.get(), "TestPool");
...@@ -71,14 +81,20 @@ TEST( ...@@ -71,14 +81,20 @@ TEST(
<< "Deadlock detector not yet registered"; << "Deadlock detector not yet registered";
executor->addObserver(observer); executor->addObserver(observer);
deadlockDetectorFactory->baton.wait();
deadlockDetectorFactory->baton.reset();
ASSERT_EQ(1, deadlockDetectorFactory->getCounter()) ASSERT_EQ(1, deadlockDetectorFactory->getCounter())
<< "Deadlock detector must be registered by observer"; << "Deadlock detector must be registered by observer";
EXPECT_EQ(
fmt::format("TestPool:{}", thid), deadlockDetectorFactory->getName());
executor->removeObserver(observer); executor->removeObserver(observer);
ASSERT_EQ(0, deadlockDetectorFactory->getCounter()) ASSERT_EQ(0, deadlockDetectorFactory->getCounter())
<< "Removing observer must destroy deadlock detector"; << "Removing observer must destroy deadlock detector";
executor->addObserver(observer); executor->addObserver(observer);
deadlockDetectorFactory->baton.wait();
deadlockDetectorFactory->baton.reset();
ASSERT_EQ(1, deadlockDetectorFactory->getCounter()) ASSERT_EQ(1, deadlockDetectorFactory->getCounter())
<< "Deadlock detector must be registered by observer"; << "Deadlock detector must be registered by observer";
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment