Commit d7f1adb6 authored by Wez Furlong's avatar Wez Furlong Committed by Tudor Bosman

use recursive_mutex to protect State

Summary:
maelstrom destructs a Promise during an indirect call from
maybeCallback and deadlocks on itself unless we use a recursive mutex.

There may be a smarter way to proceed but at the moment I can't build
and deploy a package from trunk because the service is non-functional
:-/

Test Plan:
run

```
fbconfig -r folly/wangle messaging/maelstrom
fbmake runtests
```

Reviewed By: hannesr@fb.com

Subscribers: fugalh, fsilva

FB internal diff: D1428504
parent df377ed2
...@@ -57,7 +57,7 @@ class State { ...@@ -57,7 +57,7 @@ class State {
template <typename F> template <typename F>
void setCallback(F func) { void setCallback(F func) {
{ {
std::lock_guard<std::mutex> lock(mutex_); std::lock_guard<decltype(mutex_)> lock(mutex_);
if (callback_) { if (callback_) {
throw std::logic_error("setCallback called twice"); throw std::logic_error("setCallback called twice");
...@@ -71,7 +71,7 @@ class State { ...@@ -71,7 +71,7 @@ class State {
void fulfil(Try<T>&& t) { void fulfil(Try<T>&& t) {
{ {
std::lock_guard<std::mutex> lock(mutex_); std::lock_guard<decltype(mutex_)> lock(mutex_);
if (ready()) { if (ready()) {
throw std::logic_error("fulfil called twice"); throw std::logic_error("fulfil called twice");
...@@ -122,13 +122,13 @@ class State { ...@@ -122,13 +122,13 @@ class State {
} }
void deactivate() { void deactivate() {
std::lock_guard<std::mutex> lock(mutex_); std::lock_guard<decltype(mutex_)> lock(mutex_);
active_ = false; active_ = false;
} }
void activate() { void activate() {
{ {
std::lock_guard<std::mutex> lock(mutex_); std::lock_guard<decltype(mutex_)> lock(mutex_);
active_ = true; active_ = true;
} }
maybeCallback(); maybeCallback();
...@@ -136,7 +136,7 @@ class State { ...@@ -136,7 +136,7 @@ class State {
private: private:
void maybeCallback() { void maybeCallback() {
std::lock_guard<std::mutex> lock(mutex_); std::lock_guard<decltype(mutex_)> lock(mutex_);
if (!calledBack_ && if (!calledBack_ &&
value_ && callback_ && active_) { value_ && callback_ && active_) {
// TODO we should probably try/catch here // TODO we should probably try/catch here
...@@ -148,7 +148,7 @@ class State { ...@@ -148,7 +148,7 @@ class State {
void detachOne() { void detachOne() {
bool shouldDelete; bool shouldDelete;
{ {
std::lock_guard<std::mutex> lock(mutex_); std::lock_guard<decltype(mutex_)> lock(mutex_);
detached_++; detached_++;
assert(detached_ == 1 || detached_ == 2); assert(detached_ == 1 || detached_ == 2);
shouldDelete = (detached_ == 2); shouldDelete = (detached_ == 2);
...@@ -170,7 +170,7 @@ class State { ...@@ -170,7 +170,7 @@ class State {
// this lock isn't meant to protect all accesses to members, only the ones // this lock isn't meant to protect all accesses to members, only the ones
// that need to be threadsafe: the act of setting value_ and callback_, and // that need to be threadsafe: the act of setting value_ and callback_, and
// seeing if they are set and whether we should then continue. // seeing if they are set and whether we should then continue.
std::mutex mutex_; std::recursive_mutex mutex_;
}; };
template <typename... Ts> template <typename... Ts>
......
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