Go back to a regular mutex
authorHans Fugal <fugalh@fb.com>
Thu, 9 Oct 2014 23:27:12 +0000 (16:27 -0700)
committerAndrii Grynenko <andrii@fb.com>
Wed, 15 Oct 2014 00:59:46 +0000 (17:59 -0700)
commit550b907b7c5b104d2fda6a59d0debba49d3f056d
tree0a028ac495754ef00fcfb124d66071fbbd4c8dbb
parent58132d0716e0326285520aec7e5dc42463d8851d
Go back to a regular mutex

Summary:
D1428504 changed the `detail::State` mutex to be a recursive mutex to get around deadlock in our project, where a promise was being freed within a callback, racing with the mechanics happening while fulfilling the promise in the first place. At least, that's what seemed to be happening. I couldn't easily pull it into gdb because it's a python test.

I made my own test to repro, and it did in fact deadlock:

TEST(Future, DISABLED_promiseDestructedDuringCallback) {
auto p = folly::make_unique<Promise<void>>();
p->getFuture().then([&](Try<void>&&) {
// shouldn't deadlock.
p.reset();
});
p->setValue();
}

However, although this code fixes our project it does not fix this code, which still fails (not with a deadlock, but with a promiseAlreadyFulfilled exception). So whatever our project is doing is a bit more intricate.
I'm not convinced that even allowing this is ok - so I suspect out project is doing something bad. However, I also think it's probably bad to hold the lock while doing the callback so I am presenting this as a fix/compromise.

Test Plan:
unit tests

Reviewed By: hannesr@fb.com

Subscribers: net-systems@, fugalh, exa, njormrod, fsilva, davejwatson

FB internal diff: D1604829

Blame Revision: D1428504
folly/wangle/detail/State.h