From 8477e25603f131c0400b6964609c76b438826788 Mon Sep 17 00:00:00 2001 From: Aaryaman Sagar Date: Sun, 7 Mar 2021 22:28:36 -0800 Subject: [PATCH] Explicitly qualify atomic_wait and atomic_notify function calls Summary: `std::atomic_wait()` and `std::atomic_notify_one()` are marked as unavailable. Not really sure what that means, but it seems to be breaking some open source builds https://github.com/facebook/folly/issues/1527. Explicitly qualify the calls into those functions to try and fix the build break. Since we cannot conditionally import either of the above (because they are marked as unavailable), we can't rely on the standard implementations. To prevent ADL from kicking in when the standard-library defines these, we fully qualify these and use `tag_invoke` for the customization points used in tests. Reviewed By: davidtgoldblatt Differential Revision: D26742072 fbshipit-source-id: 9f44bbfd37530f5ecffa3e03d673cfb1df953299 --- folly/lang/CustomizationPoint.h | 4 +- .../synchronization/AtomicNotification-inl.h | 27 ++++---- folly/synchronization/AtomicNotification.h | 69 ++++++++++++++++--- folly/synchronization/DistributedMutex-inl.h | 7 +- .../test/DistributedMutexTest.cpp | 6 +- folly/test/DeterministicSchedule.h | 14 ++-- 6 files changed, 91 insertions(+), 36 deletions(-) diff --git a/folly/lang/CustomizationPoint.h b/folly/lang/CustomizationPoint.h index f48d4ca1213..3474dc518bf 100644 --- a/folly/lang/CustomizationPoint.h +++ b/folly/lang/CustomizationPoint.h @@ -50,7 +50,7 @@ namespace folly { // Using 'auto' for non-type template parameters is only possible from C++17 -#if __cplusplus >= 201703L +#if FOLLY_CPLUSPLUS >= 201703L // cpo_t // @@ -71,6 +71,6 @@ namespace folly { template using cpo_t = std::decay_t; -#endif // __cplusplus >= 201703L +#endif // FOLLY_CPLUSPLUS >= 201703L } // namespace folly diff --git a/folly/synchronization/AtomicNotification-inl.h b/folly/synchronization/AtomicNotification-inl.h index 77b2d264ee0..183ecdf1c05 100644 --- a/folly/synchronization/AtomicNotification-inl.h +++ b/folly/synchronization/AtomicNotification-inl.h @@ -16,9 +16,6 @@ #pragma once -#include -#include - #include #include @@ -120,31 +117,31 @@ void atomic_notify_all_impl(const Atom* atomic) { return UnparkControl::RemoveContinue; }); } -} // namespace atomic_notification -} // namespace detail template -void atomic_wait(const std::atomic* atomic, Integer expected) { - detail::atomic_notification::atomic_wait_impl(atomic, expected); +void tag_invoke( + atomic_wait_fn, const std::atomic* atomic, Integer expected) { + atomic_wait_impl(atomic, expected); } template -std::cv_status atomic_wait_until( +std::cv_status tag_invoke( + atomic_wait_until_fn, const std::atomic* atomic, Integer expected, const std::chrono::time_point& deadline) { - return detail::atomic_notification::atomic_wait_until_impl( - atomic, expected, deadline); + return atomic_wait_until_impl(atomic, expected, deadline); } template -void atomic_notify_one(const std::atomic* atomic) { - detail::atomic_notification::atomic_notify_one_impl(atomic); +void tag_invoke(atomic_notify_one_fn, const std::atomic* atomic) { + atomic_notify_one_impl(atomic); } template -void atomic_notify_all(const std::atomic* atomic) { - detail::atomic_notification::atomic_notify_all_impl(atomic); +void tag_invoke(atomic_notify_all_fn, const std::atomic* atomic) { + atomic_notify_all_impl(atomic); } - +} // namespace atomic_notification +} // namespace detail } // namespace folly diff --git a/folly/synchronization/AtomicNotification.h b/folly/synchronization/AtomicNotification.h index b1ebbd2408d..2448aef0e49 100644 --- a/folly/synchronization/AtomicNotification.h +++ b/folly/synchronization/AtomicNotification.h @@ -45,20 +45,67 @@ namespace folly { * is identical to futexWaitUntil() and returns std::cv_status */ // mimic: std::atomic_wait, p1135r0 -template -void atomic_wait(const std::atomic* atomic, Integer old); -template -std::cv_status atomic_wait_until( - const std::atomic* atomic, - Integer old, - const std::chrono::time_point& deadline); +namespace detail { +namespace atomic_notification { +struct atomic_wait_fn { + public: + template + constexpr void operator()(const Atomic* atomic, Integer integer) const { + tag_invoke(*this, atomic, integer); + } +}; +} // namespace atomic_notification +} // namespace detail +constexpr inline auto atomic_wait = + detail::atomic_notification::atomic_wait_fn{}; + +// mimic: std::atomic_wait_until, p1135r0 +namespace detail { +namespace atomic_notification { +struct atomic_wait_until_fn { + public: + template + constexpr std::cv_status operator()( + const Atomic* atomic, + Integer old, + const std::chrono::time_point& deadline) const { + return tag_invoke(*this, atomic, old, deadline); + } +}; +} // namespace atomic_notification +} // namespace detail +constexpr inline auto atomic_wait_until = + detail::atomic_notification::atomic_wait_until_fn{}; // mimic: std::atomic_notify_one, p1135r0 -template -void atomic_notify_one(const std::atomic* atomic); +namespace detail { +namespace atomic_notification { +struct atomic_notify_one_fn { + public: + template + constexpr void operator()(const Atomic* atomic) const { + tag_invoke(*this, atomic); + } +}; +} // namespace atomic_notification +} // namespace detail +constexpr inline auto atomic_notify_one = + detail::atomic_notification::atomic_notify_one_fn{}; + // mimic: std::atomic_notify_all, p1135r0 -template -void atomic_notify_all(const std::atomic* atomic); +namespace detail { +namespace atomic_notification { +struct atomic_notify_all_fn { + public: + template + constexpr void operator()(Atomic* atomic) const { + tag_invoke(*this, atomic); + } +}; +} // namespace atomic_notification +} // namespace detail +constexpr inline auto atomic_notify_all = + detail::atomic_notification::atomic_notify_all_fn{}; // mimic: std::atomic_uint_fast_wait_t, p1135r0 using atomic_uint_fast_wait_t = std::atomic; diff --git a/folly/synchronization/DistributedMutex-inl.h b/folly/synchronization/DistributedMutex-inl.h index 68c2b208052..9b981710112 100644 --- a/folly/synchronization/DistributedMutex-inl.h +++ b/folly/synchronization/DistributedMutex-inl.h @@ -1040,7 +1040,7 @@ inline void recordTimedWaiterAndClearTimedBit( template void wakeTimedWaiters(Atomic* state, bool timedWaiters) { if (UNLIKELY(timedWaiters)) { - atomic_notify_one(state); + folly::atomic_notify_one(state); // evade ADL } } @@ -1665,7 +1665,10 @@ auto timedLock(Atomic& state, Deadline deadline, MakeProxy proxy) { // wait on the futex until signalled, if we get a timeout, the try_lock // fails - auto result = atomic_wait_until(&state, previous | data, deadline); + auto result = folly::atomic_wait_until( // evade ADL + &state, + previous | data, + deadline); if (result == std::cv_status::timeout) { return proxy(nullptr, std::uintptr_t{0}, false); } diff --git a/folly/synchronization/test/DistributedMutexTest.cpp b/folly/synchronization/test/DistributedMutexTest.cpp index 99f3a2f51a5..ae49a410adf 100644 --- a/folly/synchronization/test/DistributedMutexTest.cpp +++ b/folly/synchronization/test/DistributedMutexTest.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -174,7 +175,8 @@ folly::detail::FutexResult futexWaitImpl( } template -std::cv_status atomic_wait_until( +std::cv_status tag_invoke( + cpo_t, const ManualAtomic*, std::uintptr_t, const std::chrono::time_point&) { @@ -182,7 +184,7 @@ std::cv_status atomic_wait_until( return std::cv_status::no_timeout; } -void atomic_notify_one(const ManualAtomic*) { +void tag_invoke(cpo_t, const ManualAtomic*) { ManualSchedule::beforeSharedAccess(); } } // namespace test diff --git a/folly/test/DeterministicSchedule.h b/folly/test/DeterministicSchedule.h index 04501cf4ff6..a7d5b4fc7d4 100644 --- a/folly/test/DeterministicSchedule.h +++ b/folly/test/DeterministicSchedule.h @@ -33,6 +33,8 @@ #include #include #include +#include +#include #include #include @@ -684,18 +686,22 @@ detail::FutexResult deterministicFutexWaitImpl( * waits and wakes */ template -void atomic_wait(const DeterministicAtomic*, Integer) {} +void tag_invoke( + cpo_t, const DeterministicAtomic*, Integer) {} template -std::cv_status atomic_wait_until( +std::cv_status tag_invoke( + cpo_t, const DeterministicAtomic*, Integer, const std::chrono::time_point&) { return std::cv_status::no_timeout; } template -void atomic_notify_one(const DeterministicAtomic*) {} +void tag_invoke(cpo_t, const DeterministicAtomic*) { +} template -void atomic_notify_all(const DeterministicAtomic*) {} +void tag_invoke(cpo_t, const DeterministicAtomic*) { +} /** * DeterministicMutex is a drop-in replacement of std::mutex that