Skip to content

Commit

Permalink
introduce ImplicitSynchronized
Browse files Browse the repository at this point in the history
Summary:
`folly::Synchronized::operator->` is a footgun in three ways.

* It's easy to accidentally acquire a lock in an inner loop.
* It's easy to cause a deadlock.
* It's easy to non-atomically update independent fields under a mutex with repeated uses of `->`.

It's clear that our deprecation warnings here aren't sufficient. Uses of
`operator->` are showing up in new code.

To ease the migration and prevent new accidental uses, move the operators
into an `ImplicitSynchronized` subclass.

This work broken into three steps to ease landing incrementally:
* Introduce the ImplicitSynchronized type.
* codemod usage sites that need ImplicitSynchronized.
* Move `operator->` from Synchronized to ImplicitSynchronized.

Reviewed By: Gownta

Differential Revision: D48277297

fbshipit-source-id: a4b51e494c5ca73f88b79392694a687030a2046a
  • Loading branch information
chadaustin authored and facebook-github-bot committed Sep 12, 2023
1 parent 49cf61b commit 1385306
Showing 1 changed file with 22 additions and 0 deletions.
22 changes: 22 additions & 0 deletions folly/Synchronized.h
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,28 @@ struct Synchronized : public SynchronizedBase<
mutable Mutex mutex_;
};

/**
* Deprecated subclass of Synchronized that provides implicit locking
* via operator->. This is intended to ease migration while preventing
* accidental use of operator-> in new code.
*/
template <class T, class Mutex = SharedMutex>
struct [[deprecated(
"use Synchronized and explicit lock(), wlock(), or rlock() instead")]] ImplicitSynchronized
: Synchronized<T, Mutex> {
private:
using Base = Synchronized<T, Mutex>;

public:
using LockedPtr = typename Base::LockedPtr;
using ConstLockedPtr = typename Base::ConstLockedPtr;
using DataType = typename Base::DataType;
using MutexType = typename Base::MutexType;

using Base::Base;
using Base::operator=;
};

template <class SynchronizedType, class LockPolicy>
class ScopedUnlocker;

Expand Down

0 comments on commit 1385306

Please sign in to comment.