use recursive_mutex to protect State
authorWez Furlong <wez@fb.com>
Thu, 10 Jul 2014 18:37:17 +0000 (11:37 -0700)
committerTudor Bosman <tudorb@fb.com>
Mon, 14 Jul 2014 19:13:48 +0000 (12:13 -0700)
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

folly/wangle/detail/State.h

index 4c78d956f589ffc66f3b8cf3008541c675b12ed3..1e98e4e3bd003abfc32f7ac7af67aacb3c9b988b 100644 (file)
@@ -57,7 +57,7 @@ class State {
   template <typename F>
   void setCallback(F func) {
     {
-      std::lock_guard<std::mutex> lock(mutex_);
+      std::lock_guard<decltype(mutex_)> lock(mutex_);
 
       if (callback_) {
         throw std::logic_error("setCallback called twice");
@@ -71,7 +71,7 @@ class State {
 
   void fulfil(Try<T>&& t) {
     {
-      std::lock_guard<std::mutex> lock(mutex_);
+      std::lock_guard<decltype(mutex_)> lock(mutex_);
 
       if (ready()) {
         throw std::logic_error("fulfil called twice");
@@ -122,13 +122,13 @@ class State {
   }
 
   void deactivate() {
-    std::lock_guard<std::mutex> lock(mutex_);
+    std::lock_guard<decltype(mutex_)> lock(mutex_);
     active_ = false;
   }
 
   void activate() {
     {
-      std::lock_guard<std::mutex> lock(mutex_);
+      std::lock_guard<decltype(mutex_)> lock(mutex_);
       active_ = true;
     }
     maybeCallback();
@@ -136,7 +136,7 @@ class State {
 
  private:
   void maybeCallback() {
-    std::lock_guard<std::mutex> lock(mutex_);
+    std::lock_guard<decltype(mutex_)> lock(mutex_);
     if (!calledBack_ &&
         value_ && callback_ && active_) {
       // TODO we should probably try/catch here
@@ -148,7 +148,7 @@ class State {
   void detachOne() {
     bool shouldDelete;
     {
-      std::lock_guard<std::mutex> lock(mutex_);
+      std::lock_guard<decltype(mutex_)> lock(mutex_);
       detached_++;
       assert(detached_ == 1 || detached_ == 2);
       shouldDelete = (detached_ == 2);
@@ -170,7 +170,7 @@ class State {
   // 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
   // seeing if they are set and whether we should then continue.
-  std::mutex mutex_;
+  std::recursive_mutex mutex_;
 };
 
 template <typename... Ts>