Skip to content

Commit

Permalink
move Synchronized::operator-> into 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.

Reviewed By: Gownta

Differential Revision: D64356335

fbshipit-source-id: 8a57c64acd0dff1b2d7e384d24022d79d45707ca
  • Loading branch information
chadaustin authored and facebook-github-bot committed Nov 5, 2024
1 parent f60f6e2 commit c4f9cc9
Showing 1 changed file with 32 additions and 32 deletions.
64 changes: 32 additions & 32 deletions folly/Synchronized.h
Original file line number Diff line number Diff line change
Expand Up @@ -828,38 +828,6 @@ struct Synchronized : public SynchronizedBase<
return ConstLockedPtr(this, timeout);
}

/**
* @brief Access the datum under lock.
*
* deprecated
*
* This accessor offers a LockedPtr. In turn, LockedPtr offers
* operator-> returning a pointer to T. The operator-> keeps
* expanding until it reaches a pointer, so syncobj->foo() will lock
* the object and call foo() against it.
*
* NOTE: This API is planned to be deprecated in an upcoming diff.
* Prefer using lock(), wlock(), or rlock() instead.
*/
[[deprecated("use explicit lock(), wlock(), or rlock() instead")]] LockedPtr
operator->() {
return LockedPtr(this);
}

/**
* deprecated
*
* Obtain a ConstLockedPtr.
*
* NOTE: This API is planned to be deprecated in an upcoming diff.
* Prefer using lock(), wlock(), or rlock() instead.
*/
[[deprecated(
"use explicit lock(), wlock(), or rlock() instead")]] ConstLockedPtr
operator->() const {
return ConstLockedPtr(this);
}

/**
* @brief Acquire a LockedPtr with timeout.
*
Expand Down Expand Up @@ -1039,6 +1007,38 @@ struct [[deprecated(

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

/**
* @brief Access the datum under lock.
*
* deprecated
*
* This accessor offers a LockedPtr. In turn, LockedPtr offers
* operator-> returning a pointer to T. The operator-> keeps
* expanding until it reaches a pointer, so syncobj->foo() will lock
* the object and call foo() against it.
*
* NOTE: This API is planned to be deprecated in an upcoming diff.
* Prefer using lock(), wlock(), or rlock() instead.
*/
[[deprecated("use explicit lock(), wlock(), or rlock() instead")]] LockedPtr
operator->() {
return LockedPtr(this);
}

/**
* deprecated
*
* Obtain a ConstLockedPtr.
*
* NOTE: This API is planned to be deprecated in an upcoming diff.
* Prefer using lock(), wlock(), or rlock() instead.
*/
[[deprecated(
"use explicit lock(), wlock(), or rlock() instead")]] ConstLockedPtr
operator->() const {
return ConstLockedPtr(this);
}
};

template <class SynchronizedType, class LockPolicy>
Expand Down

0 comments on commit c4f9cc9

Please sign in to comment.