From 44d158805cf0570211f26fa7af6e5a7a580a8ef8 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Thu, 14 Nov 2019 08:04:02 +0000 Subject: [PATCH] Reimplement std::future and std::promise to workaround TSAN false positives --- include/dap/future.h | 179 ++++++++++++++++++++++++++++++++++++++++++ include/dap/session.h | 19 +++-- src/session_test.cpp | 2 + 3 files changed, 190 insertions(+), 10 deletions(-) create mode 100644 include/dap/future.h diff --git a/include/dap/future.h b/include/dap/future.h new file mode 100644 index 0000000..abc8abf --- /dev/null +++ b/include/dap/future.h @@ -0,0 +1,179 @@ +// Copyright 2019 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef dap_future_h +#define dap_future_h + +#include +#include +#include + +namespace dap { + +// internal functionality +namespace detail { +template +struct promise_state { + T val; + std::mutex mutex; + std::condition_variable cv; + bool hasVal = false; +}; +} // namespace detail + +// forward declaration +template +class promise; + +// future_status is the enumeration returned by future::wait_for and +// future::wait_until. +enum class future_status { + ready, + timeout, +}; + +// future is a minimal reimplementation of std::future, that does not suffer +// from TSAN false positives. See: +// https://gcc.gnu.org/bugzilla//show_bug.cgi?id=69204 +template +class future { + public: + using State = detail::promise_state; + + // constructors + inline future() = default; + inline future(future&&) = default; + + // valid() returns true if the future has an internal state. + bool valid() const; + + // get() blocks until the future has a valid result, and returns it. + // The future must have a valid internal state to call this method. + inline T get(); + + // wait() blocks until the future has a valid result. + // The future must have a valid internal state to call this method. + void wait() const; + + // wait_for() blocks until the future has a valid result, or the timeout is + // reached. + // The future must have a valid internal state to call this method. + template + future_status wait_for( + const std::chrono::duration& timeout) const; + + // wait_until() blocks until the future has a valid result, or the timeout is + // reached. + // The future must have a valid internal state to call this method. + template + future_status wait_until( + const std::chrono::time_point& timeout) const; + + private: + friend promise; + future(const future&) = delete; + inline future(const std::shared_ptr& state); + + std::shared_ptr state = std::make_shared(); +}; + +template +future::future(const std::shared_ptr& state) : state(state) {} + +template +bool future::valid() const { + return state; +} + +template +T future::get() { + std::unique_lock lock(state->mutex); + state->cv.wait(lock, [&] { return state->hasVal; }); + return state->val; +} + +template +void future::wait() const { + std::unique_lock lock(state->mutex); + state->cv.wait(lock, [&] { return state->hasVal; }); +} + +template +template +future_status future::wait_for( + const std::chrono::duration& timeout) const { + std::unique_lock lock(state->mutex); + return state->cv.wait_for(lock, timeout, [&] { return state->hasVal; }) + ? future_status::ready + : future_status::timeout; +} + +template +template +future_status future::wait_until( + const std::chrono::time_point& timeout) const { + std::unique_lock lock(state->mutex); + return state->cv.wait_until(lock, timeout, [&] { return state->hasVal; }) + ? future_status::ready + : future_status::timeout; +} + +// promise is a minimal reimplementation of std::promise, that does not suffer +// from TSAN false positives. See: +// https://gcc.gnu.org/bugzilla//show_bug.cgi?id=69204 +template +class promise { + public: + // constructors + inline promise() = default; + inline promise(promise&& other) = default; + inline promise(const promise& other) = default; + + // set_value() stores value to the shared state. + // set_value() must only be called once. + inline void set_value(const T& value) const; + inline void set_value(T&& value) const; + + // get_future() returns a future sharing this promise's state. + future get_future(); + + private: + using State = detail::promise_state; + std::shared_ptr state = std::make_shared(); +}; + +template +future promise::get_future() { + return future(state); +} + +template +void promise::set_value(const T& value) const { + std::unique_lock lock(state->mutex); + state->val = value; + state->hasVal = true; + state->cv.notify_all(); +} + +template +void promise::set_value(T&& value) const { + std::unique_lock lock(state->mutex); + state->val = std::move(value); + state->hasVal = true; + state->cv.notify_all(); +} + +} // namespace dap + +#endif // dap_future_h diff --git a/include/dap/session.h b/include/dap/session.h index 056c633..f6cbb8b 100644 --- a/include/dap/session.h +++ b/include/dap/session.h @@ -15,12 +15,12 @@ #ifndef dap_session_h #define dap_session_h +#include "future.h" #include "io.h" #include "typeinfo.h" #include "typeof.h" #include -#include namespace dap { @@ -158,9 +158,9 @@ class Session { inline void registerSentHandler(F&& handler); // send() sends the request to the connected endpoint and returns a - // std::future that is assigned the request response or error. + // future that is assigned the request response or error. template > - std::future> send(const T& request); + future> send(const T& request); // send() sends the event to the connected endpoint. template > @@ -248,24 +248,23 @@ void Session::registerSentHandler(F&& handler) { } template -std::future> Session::send( - const T& request) { +future> Session::send(const T& request) { using Response = typename T::Response; - auto promise = std::make_shared>>(); + promise> promise; const TypeInfo* typeinfo = TypeOf::type(); auto sent = send(typeinfo, &request, [=](const void* result, const Error* error) { if (error != nullptr) { - promise->set_value(ResponseOrError(*error)); + promise.set_value(ResponseOrError(*error)); } else { - promise->set_value(ResponseOrError( + promise.set_value(ResponseOrError( *reinterpret_cast(result))); } }); if (!sent) { - promise->set_value(Error("Failed to send request")); + promise.set_value(Error("Failed to send request")); } - return promise->get_future(); + return promise.get_future(); } template diff --git a/src/session_test.cpp b/src/session_test.cpp index 04409e5..66974fa 100644 --- a/src/session_test.cpp +++ b/src/session_test.cpp @@ -22,8 +22,10 @@ #include "gtest/gtest.h" #include +#include #include #include +#include namespace dap {