Commit 30af28c6 authored by Yedidya Feldblum's avatar Yedidya Feldblum Committed by Facebook Github Bot

Fix SharedPromise move-ctor, move-assign

Summary: [Folly] Fix `SharedPromise` move-ctor, move-assign. Some fields really should be zero'd.

Reviewed By: davidtgoldblatt

Differential Revision: D13152287

fbshipit-source-id: 1ab2da8016cbd736541a98ff32f32c30f6452209
parent 5f5230ff
......@@ -21,15 +21,15 @@ namespace folly {
template <class T>
size_t SharedPromise<T>::size() {
std::lock_guard<std::mutex> g(mutex_);
return size_;
return size_.value;
}
template <class T>
SemiFuture<T> SharedPromise<T>::getSemiFuture() {
std::lock_guard<std::mutex> g(mutex_);
size_++;
if (hasValue_) {
return makeFuture<T>(Try<T>(try_));
size_.value++;
if (hasResult()) {
return makeFuture<T>(Try<T>(try_.value));
} else {
promises_.emplace_back();
if (interruptHandler_) {
......@@ -60,7 +60,7 @@ template <class T>
void SharedPromise<T>::setInterruptHandler(
std::function<void(exception_wrapper const&)> fn) {
std::lock_guard<std::mutex> g(mutex_);
if (hasValue_) {
if (hasResult()) {
return;
}
interruptHandler_ = fn;
......@@ -87,23 +87,22 @@ void SharedPromise<T>::setTry(Try<T>&& t) {
{
std::lock_guard<std::mutex> g(mutex_);
if (hasValue_) {
if (hasResult()) {
throw_exception<PromiseAlreadySatisfied>();
}
hasValue_ = true;
try_ = std::move(t);
try_.value = std::move(t);
promises.swap(promises_);
}
for (auto& p : promises) {
p.setTry(Try<T>(try_));
p.setTry(Try<T>(try_.value));
}
}
template <class T>
bool SharedPromise<T>::isFulfilled() {
std::lock_guard<std::mutex> g(mutex_);
return hasValue_;
return hasResult();
}
} // namespace folly
......@@ -109,10 +109,29 @@ class SharedPromise {
}
};
template <typename V>
struct Defaulted {
using Noexcept = StrictConjunction<
std::is_nothrow_default_constructible<V>,
std::is_nothrow_move_constructible<V>,
std::is_nothrow_move_assignable<V>>;
V value{V()};
Defaulted() = default;
Defaulted(Defaulted&& that) noexcept(Noexcept::value)
: value(std::exchange(that.value, V())) {}
Defaulted& operator=(Defaulted&& that) noexcept(Noexcept::value) {
value = std::exchange(that.value, V());
return *this;
}
};
bool hasResult() {
return try_.value.hasValue() || try_.value.hasException();
}
Mutex mutex_;
size_t size_{0};
bool hasValue_{false};
Try<T> try_;
Defaulted<size_t> size_;
Defaulted<Try<T>> try_;
std::vector<Promise<T>> promises_;
std::function<void(exception_wrapper const&)> interruptHandler_;
};
......
......@@ -114,7 +114,9 @@ TEST(SharedPromise, moveMove) {
auto f1 = p.getFuture();
auto f2 = p.getFuture();
auto p2 = std::move(p);
EXPECT_EQ(2, p2.size());
p = std::move(p2);
EXPECT_EQ(0, p2.size());
p.setValue(std::make_shared<int>(1));
}
......@@ -133,6 +135,7 @@ TEST(SharedPromise, isFulfilled) {
EXPECT_TRUE(p2.isFulfilled());
p = std::move(p2);
EXPECT_TRUE(p.isFulfilled());
EXPECT_FALSE(p2.isFulfilled());
}
TEST(SharedPromise, interruptHandler) {
......
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