Commit 9fb1f368 authored by Kenny Yu's avatar Kenny Yu Committed by Facebook Github Bot

folly: Fix data race in folly::Codel

Summary:
Data race reported by TSAN:

```
WARNING: ThreadSanitizer: data race (pid=608219)
  Read of size 1 at 0x7b5800000c29 by thread T314:
    #0 0x60b3441 in folly::Codel::overloaded(std::chrono::duration<long, std::ratio<1l, 1000000000l> >) ./folly/executors/Codel.cpp:76
    #1 0x5c1222 in apache::thrift::concurrency::ThreadManager::ImplT<folly::LifoSemImpl<std::atomic, folly::Baton<std::atomic, true, true> > >::Worker<folly::LifoSemImpl<std::atomic, folly::Baton<std::atomic, true, true> > >::run() ./thrift/lib/cpp/concurrency/ThreadManager.tcc:119
    #2 0x5d803e7 in apache::thrift::concurrency::PthreadThread::threadMain(void*) ./thrift/lib/cpp/concurrency/PosixThreadFactory.cpp:200
    #3 0x619739d in __tsan_thread_start_func crtstuff.c:?

  Previous write of size 1 at 0x7b5800000c29 by thread T315:
    #0 0x60b33e4 in folly::Codel::overloaded(std::chrono::duration<long, std::ratio<1l, 1000000000l> >) ??:?
    #1 0x5c1222 in apache::thrift::concurrency::ThreadManager::ImplT<folly::LifoSemImpl<std::atomic, folly::Baton<std::atomic, true, true> > >::Worker<folly::LifoSemImpl<std::atomic, folly::Baton<std::atomic, true, true> > >::run() ./thrift/lib/cpp/concurrency/ThreadManager.tcc:119
    #2 0x5d803e7 in apache::thrift::concurrency::PthreadThread::threadMain(void*) ./thrift/lib/cpp/concurrency/PosixThreadFactory.cpp:200
    #3 0x619739d in __tsan_thread_start_func crtstuff.c:?

  Location is heap block of size 768 at 0x7b5800000c00 allocated by main thread:
    #0 0x616ab83 in operator new(unsigned long) ??:?
    #1 0x53cb92 in __gnu_cxx::new_allocator<std::_Sp_counted_ptr_inplace<apache::thrift::concurrency::SimpleThreadManager<folly::LifoSemImpl<std::atomic, folly::Baton<std::atomic, true, true> > >, std::allocator<apache::thrift::concurrency::SimpleThreadManager<folly::LifoSemImpl<std::atomic, folly::Baton<std::atomic, true, true> > > >, (__gnu_cxx::_Lock_policy)2> >::allocate(unsigned long, void const*)
    ...
```

It looks like there are multiple threads reading and writing the `overloaded_` bool. To fix it, wrap it in a `std::atomic`.

Reviewed By: yfeldblum, meyering

Differential Revision: D6149766

fbshipit-source-id: 605b29fa2f602d2ed2dfc22e46b739ef169f914e
parent d4f016d5
......@@ -87,7 +87,7 @@ class Codel {
// to reset the delay once per time period
std::atomic<bool> codelResetDelay_;
bool overloaded_;
std::atomic<bool> overloaded_;
};
} // namespace folly
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