Fix unsynchronized accesses in IOThreadPoolExecutor::getEventBase
authorYedidya Feldblum <yfeldblum@fb.com>
Wed, 1 Nov 2017 21:11:27 +0000 (14:11 -0700)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Wed, 1 Nov 2017 21:17:59 +0000 (14:17 -0700)
Summary:
[Folly] Fix unsynchronized accesses in `IOThreadPoolExecutor::getEventBase`.

`getEventBase` may be invoked concurrently from two threads - RMWs to `nextThread_` must be synchronized with each other.

`getEventBase` may be invoked concurrently with `setNumThreads` - the former's reads of `threadList_.vec_` must be synchronized with the latter's writes to it.

Reviewed By: kennyyu

Differential Revision: D6206916

fbshipit-source-id: 8bfae158effb5896ab478d0c20310293b037c892

folly/executors/IOThreadPoolExecutor.cpp
folly/executors/IOThreadPoolExecutor.h

index 7e16e80..63601fd 100644 (file)
@@ -119,11 +119,12 @@ IOThreadPoolExecutor::pickThread() {
   if (n == 0) {
     return me;
   }
-  auto thread = ths[nextThread_++ % n];
+  auto thread = ths[nextThread_.fetch_add(1, std::memory_order_relaxed) % n];
   return std::static_pointer_cast<IOThread>(thread);
 }
 
 EventBase* IOThreadPoolExecutor::getEventBase() {
+  RWSpinLock::ReadHolder r{&threadListLock_};
   return pickThread()->eventBase;
 }
 
index cf52822..b7c6688 100644 (file)
@@ -16,6 +16,8 @@
 
 #pragma once
 
+#include <atomic>
+
 #include <folly/executors/IOExecutor.h>
 #include <folly/executors/ThreadPoolExecutor.h>
 #include <folly/io/async/EventBaseManager.h>
@@ -86,7 +88,7 @@ class IOThreadPoolExecutor : public ThreadPoolExecutor, public IOExecutor {
   void stopThreads(size_t n) override;
   uint64_t getPendingTaskCountImpl(const RWSpinLock::ReadHolder&) override;
 
-  size_t nextThread_;
+  std::atomic<size_t> nextThread_;
   folly::ThreadLocal<std::shared_ptr<IOThread>> thisThread_;
   folly::EventBaseManager* eventBaseManager_;
 };