Commit d6f75f2e authored by Kirk Shoop's avatar Kirk Shoop Committed by Facebook Github Bot

make sure that both entangled implementations protect access to the data with actual locks.

fbshipit-source-id: 2d6466aa8cd4d37ead490fe046480d7eef746d29
parent 4d980ea0
...@@ -116,13 +116,13 @@ struct inline_executor_flow_single { ...@@ -116,13 +116,13 @@ struct inline_executor_flow_single {
struct shared_cancellation_factory{ struct shared_cancellation_factory{
auto operator()(){ auto operator()(){
// boolean cancellation // boolean cancellation
mi::moving_atomic<bool> stop = false; bool stop = false;
auto set_stop = [](auto& stop) { auto set_stop = [](auto& stop) {
if (!!stop) { if (!!stop) {
stop->store(true); *stop = true;
} }
}; };
return mi::shared_entangle(std::move(stop), set_stop); return mi::shared_entangle(stop, set_stop);
} }
}; };
using inline_executor_flow_single_shared = inline_executor_flow_single<shared_cancellation_factory>; using inline_executor_flow_single_shared = inline_executor_flow_single<shared_cancellation_factory>;
...@@ -130,13 +130,13 @@ using inline_executor_flow_single_shared = inline_executor_flow_single<shared_ca ...@@ -130,13 +130,13 @@ using inline_executor_flow_single_shared = inline_executor_flow_single<shared_ca
struct entangled_cancellation_factory{ struct entangled_cancellation_factory{
auto operator()(){ auto operator()(){
// boolean cancellation // boolean cancellation
mi::moving_atomic<bool> stop = false; bool stop = false;
auto set_stop = [](auto& stop) { auto set_stop = [](auto& stop) {
if (!!stop) { if (!!stop) {
stop->store(true); *stop = true;
} }
}; };
return mi::entangle(std::move(stop), set_stop); return mi::entangle(stop, set_stop);
} }
}; };
using inline_executor_flow_single_entangled = inline_executor_flow_single<entangled_cancellation_factory>; using inline_executor_flow_single_entangled = inline_executor_flow_single<entangled_cancellation_factory>;
......
...@@ -1432,9 +1432,11 @@ locked_entangled_pair<T, Dual> lock_both(entangled<T, Dual>& e){ ...@@ -1432,9 +1432,11 @@ locked_entangled_pair<T, Dual> lock_both(entangled<T, Dual>& e){
template <class T, class Dual> template <class T, class Dual>
struct shared_entangled : std::shared_ptr<T> { struct shared_entangled : std::shared_ptr<T> {
Dual* dual; Dual* dual;
std::mutex* lock;
template<class P> template<class P>
explicit shared_entangled(std::shared_ptr<P>& p, T& t, Dual& d) : std::shared_ptr<T>(p, std::addressof(t)), dual(std::addressof(d)){ explicit shared_entangled(std::shared_ptr<P>& p, T& t, Dual& d, std::mutex& l) :
std::shared_ptr<T>(p, std::addressof(t)), dual(std::addressof(d)), lock(std::addressof(l)){
} }
shared_entangled() = delete; shared_entangled() = delete;
shared_entangled(const shared_entangled&) = delete; shared_entangled(const shared_entangled&) = delete;
...@@ -1449,16 +1451,27 @@ using shared_entangled_pair = std::pair<shared_entangled<First, Second>, shared_ ...@@ -1449,16 +1451,27 @@ using shared_entangled_pair = std::pair<shared_entangled<First, Second>, shared_
template <class First, class Second> template <class First, class Second>
auto shared_entangle(First f, Second s) auto shared_entangle(First f, Second s)
-> shared_entangled_pair<First, Second> { -> shared_entangled_pair<First, Second> {
auto p = std::make_shared<std::pair<First, Second>>(std::move(f), std::move(s)); struct storage {
shared_entangled<First, Second> ef(p, p->first, p->second); storage(First&& f, Second&& s) : p((First&&) f, (Second&&) s) {}
shared_entangled<Second, First> es(p, p->second, p->first); std::tuple<First, Second> p;
std::mutex lock;
};
auto p = std::make_shared<storage>(std::move(f), std::move(s));
shared_entangled<First, Second> ef(p, std::get<0>(p->p), std::get<1>(p->p), p->lock);
shared_entangled<Second, First> es(p, std::get<1>(p->p), std::get<0>(p->p), p->lock);
return {std::move(ef), std::move(es)}; return {std::move(ef), std::move(es)};
} }
template <class T, class Dual> template <class T, class Dual>
struct locked_shared_entangled_pair : std::pair<T*, Dual*> { struct locked_shared_entangled_pair : std::pair<T*, Dual*> {
shared_entangled<T, Dual> e; shared_entangled<T, Dual> e;
~locked_shared_entangled_pair() {
if (!!e && !!e.lock) {
e.lock->unlock();
}
}
explicit locked_shared_entangled_pair(shared_entangled<T, Dual>& e) : e(std::move(e)){ explicit locked_shared_entangled_pair(shared_entangled<T, Dual>& e) : e(std::move(e)){
this->e.lock->lock();
this->first = this->e.get(); this->first = this->e.get();
this->second = this->e.dual; this->second = this->e.dual;
} }
...@@ -1475,13 +1488,6 @@ locked_shared_entangled_pair<T, Dual> lock_both(shared_entangled<T, Dual>& e){ ...@@ -1475,13 +1488,6 @@ locked_shared_entangled_pair<T, Dual> lock_both(shared_entangled<T, Dual>& e){
return locked_shared_entangled_pair<T, Dual>{e}; return locked_shared_entangled_pair<T, Dual>{e};
} }
// external synchronization required for move and delete
template<class T>
struct moving_atomic : std::atomic<T> {
using std::atomic<T>::atomic;
moving_atomic(moving_atomic&& o) : std::atomic<T>(o.load()) {}
};
} // namespace pushmi } // namespace pushmi
//#pragma once //#pragma once
// Copyright (c) 2018-present, Facebook, Inc. // Copyright (c) 2018-present, Facebook, Inc.
......
...@@ -235,9 +235,11 @@ locked_entangled_pair<T, Dual> lock_both(entangled<T, Dual>& e){ ...@@ -235,9 +235,11 @@ locked_entangled_pair<T, Dual> lock_both(entangled<T, Dual>& e){
template <class T, class Dual> template <class T, class Dual>
struct shared_entangled : std::shared_ptr<T> { struct shared_entangled : std::shared_ptr<T> {
Dual* dual; Dual* dual;
std::mutex* lock;
template<class P> template<class P>
explicit shared_entangled(std::shared_ptr<P>& p, T& t, Dual& d) : std::shared_ptr<T>(p, std::addressof(t)), dual(std::addressof(d)){ explicit shared_entangled(std::shared_ptr<P>& p, T& t, Dual& d, std::mutex& l) :
std::shared_ptr<T>(p, std::addressof(t)), dual(std::addressof(d)), lock(std::addressof(l)){
} }
shared_entangled() = delete; shared_entangled() = delete;
shared_entangled(const shared_entangled&) = delete; shared_entangled(const shared_entangled&) = delete;
...@@ -252,16 +254,27 @@ using shared_entangled_pair = std::pair<shared_entangled<First, Second>, shared_ ...@@ -252,16 +254,27 @@ using shared_entangled_pair = std::pair<shared_entangled<First, Second>, shared_
template <class First, class Second> template <class First, class Second>
auto shared_entangle(First f, Second s) auto shared_entangle(First f, Second s)
-> shared_entangled_pair<First, Second> { -> shared_entangled_pair<First, Second> {
auto p = std::make_shared<std::pair<First, Second>>(std::move(f), std::move(s)); struct storage {
shared_entangled<First, Second> ef(p, p->first, p->second); storage(First&& f, Second&& s) : p((First&&) f, (Second&&) s) {}
shared_entangled<Second, First> es(p, p->second, p->first); std::tuple<First, Second> p;
std::mutex lock;
};
auto p = std::make_shared<storage>(std::move(f), std::move(s));
shared_entangled<First, Second> ef(p, std::get<0>(p->p), std::get<1>(p->p), p->lock);
shared_entangled<Second, First> es(p, std::get<1>(p->p), std::get<0>(p->p), p->lock);
return {std::move(ef), std::move(es)}; return {std::move(ef), std::move(es)};
} }
template <class T, class Dual> template <class T, class Dual>
struct locked_shared_entangled_pair : std::pair<T*, Dual*> { struct locked_shared_entangled_pair : std::pair<T*, Dual*> {
shared_entangled<T, Dual> e; shared_entangled<T, Dual> e;
~locked_shared_entangled_pair() {
if (!!e && !!e.lock) {
e.lock->unlock();
}
}
explicit locked_shared_entangled_pair(shared_entangled<T, Dual>& e) : e(std::move(e)){ explicit locked_shared_entangled_pair(shared_entangled<T, Dual>& e) : e(std::move(e)){
this->e.lock->lock();
this->first = this->e.get(); this->first = this->e.get();
this->second = this->e.dual; this->second = this->e.dual;
} }
...@@ -278,11 +291,4 @@ locked_shared_entangled_pair<T, Dual> lock_both(shared_entangled<T, Dual>& e){ ...@@ -278,11 +291,4 @@ locked_shared_entangled_pair<T, Dual> lock_both(shared_entangled<T, Dual>& e){
return locked_shared_entangled_pair<T, Dual>{e}; return locked_shared_entangled_pair<T, Dual>{e};
} }
// external synchronization required for move and delete
template<class T>
struct moving_atomic : std::atomic<T> {
using std::atomic<T>::atomic;
moving_atomic(moving_atomic&& o) : std::atomic<T>(o.load()) {}
};
} // namespace pushmi } // namespace pushmi
...@@ -28,13 +28,13 @@ SCENARIO("flow single immediate cancellation", "[flow][deferred]") { ...@@ -28,13 +28,13 @@ SCENARIO("flow single immediate cancellation", "[flow][deferred]") {
GIVEN("A flow single deferred") { GIVEN("A flow single deferred") {
auto f = mi::MAKE(flow_single_deferred)([&](auto out) { auto f = mi::MAKE(flow_single_deferred)([&](auto out) {
// boolean cancellation // boolean cancellation
mi::moving_atomic<bool> stop = false; bool stop = false;
auto set_stop = [](auto& stop) { auto set_stop = [](auto& stop) {
if (!!stop) { if (!!stop) {
stop->store(true); *stop = true;
} }
}; };
auto tokens = mi::shared_entangle(std::move(stop), set_stop); auto tokens = mi::shared_entangle(stop, set_stop);
using Stopper = decltype(tokens.second); using Stopper = decltype(tokens.second);
struct Data : mi::none<> { struct Data : mi::none<> {
...@@ -110,13 +110,13 @@ SCENARIO("flow single cancellation trampoline", "[flow][deferred]") { ...@@ -110,13 +110,13 @@ SCENARIO("flow single cancellation trampoline", "[flow][deferred]") {
GIVEN("A flow single deferred") { GIVEN("A flow single deferred") {
auto f = mi::MAKE(flow_single_deferred)([&](auto out) { auto f = mi::MAKE(flow_single_deferred)([&](auto out) {
// boolean cancellation // boolean cancellation
mi::moving_atomic<bool> stop = false; bool stop = false;
auto set_stop = [](auto& stop) { auto set_stop = [](auto& stop) {
if (!!stop) { if (!!stop) {
stop->store(true); *stop = true;
} }
}; };
auto tokens = mi::shared_entangle(std::move(stop), set_stop); auto tokens = mi::shared_entangle(stop, set_stop);
using Stopper = decltype(tokens.second); using Stopper = decltype(tokens.second);
struct Data : mi::none<> { struct Data : mi::none<> {
...@@ -222,13 +222,13 @@ SCENARIO("flow single shared cancellation new thread", "[flow][deferred]") { ...@@ -222,13 +222,13 @@ SCENARIO("flow single shared cancellation new thread", "[flow][deferred]") {
GIVEN("A flow single deferred") { GIVEN("A flow single deferred") {
auto f = mi::MAKE(flow_single_deferred)([&](auto out) { auto f = mi::MAKE(flow_single_deferred)([&](auto out) {
// boolean cancellation // boolean cancellation
mi::moving_atomic<bool> stop = false; bool stop = false;
auto set_stop = [](auto& stop) { auto set_stop = [](auto& stop) {
if (!!stop) { if (!!stop) {
stop->store(true); *stop = true;
} }
}; };
auto tokens = mi::shared_entangle(std::move(stop), set_stop); auto tokens = mi::shared_entangle(stop, set_stop);
using Stopper = decltype(tokens.second); using Stopper = decltype(tokens.second);
struct Data : mi::none<> { struct Data : mi::none<> {
...@@ -392,13 +392,13 @@ SCENARIO("flow single entangled cancellation new thread", "[flow][deferred]") { ...@@ -392,13 +392,13 @@ SCENARIO("flow single entangled cancellation new thread", "[flow][deferred]") {
GIVEN("A flow single deferred") { GIVEN("A flow single deferred") {
auto f = mi::MAKE(flow_single_deferred)([&](auto out) { auto f = mi::MAKE(flow_single_deferred)([&](auto out) {
// boolean cancellation // boolean cancellation
mi::moving_atomic<bool> stop = false; bool stop = false;
auto set_stop = [](auto& stop) { auto set_stop = [](auto& stop) {
if (!!stop) { if (!!stop) {
stop->store(true); *stop = true;
} }
}; };
auto tokens = mi::entangle(std::move(stop), set_stop); auto tokens = mi::entangle(stop, set_stop);
using Stopper = decltype(tokens.second); using Stopper = decltype(tokens.second);
struct Data : mi::none<> { struct Data : mi::none<> {
......
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