Commit 816d933c authored by Chad Austin's avatar Chad Austin Committed by Facebook Github Bot

remove uses of Synchronized::operator->

Summary:
Synchronized::operator-> is dangerous because it's very easy to
implicitly acquire locks in ways that don't form a coherent locking
model. Replace uses of it in folly in preparation for removing or
marking the function deprecated later on.

Reviewed By: yfeldblum

Differential Revision: D17862465

fbshipit-source-id: 45d3b0d738941c3faa6d73418e79dcb8a1259e98
parent 4269fd6e
...@@ -78,7 +78,7 @@ class CompressionContextPool { ...@@ -78,7 +78,7 @@ class CompressionContextPool {
private: private:
void add(InternalRef ptr) { void add(InternalRef ptr) {
DCHECK(ptr); DCHECK(ptr);
stack_->push_back(std::move(ptr)); stack_.wlock()->push_back(std::move(ptr));
} }
Creator creator_; Creator creator_;
......
...@@ -123,12 +123,12 @@ GlobalThreadPoolList& GlobalThreadPoolList::instance() { ...@@ -123,12 +123,12 @@ GlobalThreadPoolList& GlobalThreadPoolList::instance() {
void GlobalThreadPoolList::registerThreadPool( void GlobalThreadPoolList::registerThreadPool(
ThreadPoolListHook* threadPoolId, ThreadPoolListHook* threadPoolId,
std::string name) { std::string name) {
globalListImpl_->registerThreadPool(threadPoolId, name); globalListImpl_.wlock()->registerThreadPool(threadPoolId, name);
} }
void GlobalThreadPoolList::unregisterThreadPool( void GlobalThreadPoolList::unregisterThreadPool(
ThreadPoolListHook* threadPoolId) { ThreadPoolListHook* threadPoolId) {
globalListImpl_->unregisterThreadPool(threadPoolId); globalListImpl_.wlock()->unregisterThreadPool(threadPoolId);
} }
void GlobalThreadPoolList::registerThreadPoolThread( void GlobalThreadPoolList::registerThreadPoolThread(
...@@ -137,7 +137,7 @@ void GlobalThreadPoolList::registerThreadPoolThread( ...@@ -137,7 +137,7 @@ void GlobalThreadPoolList::registerThreadPoolThread(
DCHECK(!threadHook_); DCHECK(!threadHook_);
threadHook_.reset(make_unique<ThreadListHook>(threadPoolId, threadId)); threadHook_.reset(make_unique<ThreadListHook>(threadPoolId, threadId));
globalListImpl_->registerThreadPoolThread(threadPoolId, threadId); globalListImpl_.wlock()->registerThreadPoolThread(threadPoolId, threadId);
} }
void GlobalThreadPoolList::unregisterThreadPoolThread( void GlobalThreadPoolList::unregisterThreadPoolThread(
...@@ -145,7 +145,7 @@ void GlobalThreadPoolList::unregisterThreadPoolThread( ...@@ -145,7 +145,7 @@ void GlobalThreadPoolList::unregisterThreadPoolThread(
std::thread::id threadId) { std::thread::id threadId) {
(void)threadPoolId; (void)threadPoolId;
(void)threadId; (void)threadId;
globalListImpl_->unregisterThreadPoolThread(threadPoolId, threadId); globalListImpl_.wlock()->unregisterThreadPoolThread(threadPoolId, threadId);
} }
void GlobalThreadPoolListImpl::registerThreadPool( void GlobalThreadPoolListImpl::registerThreadPool(
......
...@@ -45,7 +45,7 @@ ThreadPoolExecutor::ThreadPoolExecutor( ...@@ -45,7 +45,7 @@ ThreadPoolExecutor::ThreadPoolExecutor(
threadPoolHook_("folly::ThreadPoolExecutor"), threadPoolHook_("folly::ThreadPoolExecutor"),
minThreads_(minThreads), minThreads_(minThreads),
threadTimeout_(FLAGS_threadtimeout_ms) { threadTimeout_(FLAGS_threadtimeout_ms) {
getSyncVecThreadPoolExecutors()->push_back(this); getSyncVecThreadPoolExecutors().wlock()->push_back(this);
} }
ThreadPoolExecutor::~ThreadPoolExecutor() { ThreadPoolExecutor::~ThreadPoolExecutor() {
......
...@@ -23,8 +23,8 @@ ...@@ -23,8 +23,8 @@
using namespace folly::coro; using namespace folly::coro;
SharedMutexFair::~SharedMutexFair() { SharedMutexFair::~SharedMutexFair() {
assert(state_->lockedFlagAndReaderCount_ == kUnlocked); assert(state_.lock()->lockedFlagAndReaderCount_ == kUnlocked);
assert(state_->waitersHead_ == nullptr); assert(state_.lock()->waitersHead_ == nullptr);
} }
bool SharedMutexFair::try_lock() noexcept { bool SharedMutexFair::try_lock() noexcept {
......
...@@ -43,7 +43,7 @@ using ExceptionStatsHolderType = ...@@ -43,7 +43,7 @@ using ExceptionStatsHolderType =
struct ExceptionStatsStorage { struct ExceptionStatsStorage {
void appendTo(ExceptionStatsHolderType& data) { void appendTo(ExceptionStatsHolderType& data) {
ExceptionStatsHolderType tempHolder; ExceptionStatsHolderType tempHolder;
statsHolder->swap(tempHolder); statsHolder.wlock()->swap(tempHolder);
for (const auto& myData : tempHolder) { for (const auto& myData : tempHolder) {
auto inserted = data.insert(myData); auto inserted = data.insert(myData);
......
...@@ -29,11 +29,11 @@ class ObserverCreatorContext { ...@@ -29,11 +29,11 @@ class ObserverCreatorContext {
template <typename... Args> template <typename... Args>
ObserverCreatorContext(Args&&... args) ObserverCreatorContext(Args&&... args)
: observable_(std::forward<Args>(args)...) { : observable_(std::forward<Args>(args)...) {
state_->updateValue(Traits::get(observable_)); state_.unsafeGetUnlocked().updateValue(Traits::get(observable_));
} }
~ObserverCreatorContext() { ~ObserverCreatorContext() {
if (state_->value) { if (state_.unsafeGetUnlocked().value) {
Traits::unsubscribe(observable_); Traits::unsubscribe(observable_);
} }
} }
......
...@@ -37,7 +37,7 @@ SysArena* ThreadCachedArena::allocateThreadLocalArena() { ...@@ -37,7 +37,7 @@ SysArena* ThreadCachedArena::allocateThreadLocalArena() {
} }
void ThreadCachedArena::zombify(SysArena&& arena) { void ThreadCachedArena::zombify(SysArena&& arena) {
zombies_->merge(std::move(arena)); zombies_.wlock()->merge(std::move(arena));
} }
size_t ThreadCachedArena::totalSize() const { size_t ThreadCachedArena::totalSize() const {
...@@ -45,7 +45,7 @@ size_t ThreadCachedArena::totalSize() const { ...@@ -45,7 +45,7 @@ size_t ThreadCachedArena::totalSize() const {
for (const auto& arena : arena_.accessAllThreads()) { for (const auto& arena : arena_.accessAllThreads()) {
result += arena.totalSize(); result += arena.totalSize();
} }
result += zombies_->totalSize() - sizeof(SysArena); result += zombies_.rlock()->totalSize() - sizeof(SysArena);
return result; return result;
} }
......
...@@ -91,7 +91,7 @@ class ThreadCachedLists : public ThreadCachedListsBase { ...@@ -91,7 +91,7 @@ class ThreadCachedLists : public ThreadCachedListsBase {
TLHead(ThreadCachedLists* parent) : parent_(parent) {} TLHead(ThreadCachedLists* parent) : parent_(parent) {}
~TLHead() { ~TLHead() {
parent_->ghead_->splice(*this); parent_->ghead_.wlock()->splice(*this);
} }
}; };
......
...@@ -461,16 +461,16 @@ template <class Mutex> ...@@ -461,16 +461,16 @@ template <class Mutex>
void testDeprecated() { void testDeprecated() {
folly::Synchronized<std::vector<int>, Mutex> obj; folly::Synchronized<std::vector<int>, Mutex> obj;
obj->resize(1000); obj.contextualLock()->resize(1000);
auto obj2 = obj; auto obj2 = obj;
EXPECT_EQ(1000, obj2->size()); EXPECT_EQ(1000, obj2.contextualLock()->size());
SYNCHRONIZED(obj) { SYNCHRONIZED(obj) {
obj.push_back(10); obj.push_back(10);
EXPECT_EQ(1001, obj.size()); EXPECT_EQ(1001, obj.size());
EXPECT_EQ(10, obj.back()); EXPECT_EQ(10, obj.back());
EXPECT_EQ(1000, obj2->size()); EXPECT_EQ(1000, obj2.contextualLock()->size());
} }
SYNCHRONIZED_CONST(obj) { SYNCHRONIZED_CONST(obj) {
...@@ -481,9 +481,9 @@ void testDeprecated() { ...@@ -481,9 +481,9 @@ void testDeprecated() {
lockedObj.front() = 2; lockedObj.front() = 2;
} }
EXPECT_EQ(1001, obj->size()); EXPECT_EQ(1001, obj.contextualLock()->size());
EXPECT_EQ(10, obj->back()); EXPECT_EQ(10, obj.contextualLock()->back());
EXPECT_EQ(1000, obj2->size()); EXPECT_EQ(1000, obj2.contextualLock()->size());
EXPECT_EQ(FB_ARG_2_OR_1(1, 2), 2); EXPECT_EQ(FB_ARG_2_OR_1(1, 2), 2);
EXPECT_EQ(FB_ARG_2_OR_1(1), 1); EXPECT_EQ(FB_ARG_2_OR_1(1), 1);
...@@ -767,8 +767,8 @@ void testTimedSynchronized() { ...@@ -767,8 +767,8 @@ void testTimedSynchronized() {
folly::Synchronized<uint64_t, Mutex> numTimeouts; folly::Synchronized<uint64_t, Mutex> numTimeouts;
auto worker = [&](size_t threadIdx) { auto worker = [&](size_t threadIdx) {
// Test operator-> // Test contextualLock()
v->push_back(2 * threadIdx); v.contextualLock()->push_back(2 * threadIdx);
// Aaand test the TIMED_SYNCHRONIZED macro // Aaand test the TIMED_SYNCHRONIZED macro
for (;;) { for (;;) {
...@@ -814,8 +814,8 @@ void testTimedSynchronizedWithConst() { ...@@ -814,8 +814,8 @@ void testTimedSynchronizedWithConst() {
folly::Synchronized<uint64_t, Mutex> numTimeouts; folly::Synchronized<uint64_t, Mutex> numTimeouts;
auto worker = [&](size_t threadIdx) { auto worker = [&](size_t threadIdx) {
// Test operator-> // Test contextualLock()
v->push_back(threadIdx); v.contextualLock()->push_back(threadIdx);
// Test TIMED_SYNCHRONIZED_CONST // Test TIMED_SYNCHRONIZED_CONST
for (;;) { for (;;) {
......
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