(wangle) fix race in Core::detachOne()
authorHans Fugal <fugalh@fb.com>
Mon, 27 Oct 2014 15:53:23 +0000 (08:53 -0700)
committerdcsommer <dcsommer@fb.com>
Wed, 29 Oct 2014 23:05:33 +0000 (16:05 -0700)
commiteec32a760c6597c7053fac69d9c844b9ff422677
tree73d97f442c3b445066bbe9bec0d284243e1ecc62
parent3d6fc64fce1e416b36a792f540d51306b08238e4
(wangle) fix race in Core::detachOne()

Summary:
In D1618240 I introduced a race condition in `detachOne()`, where `detached_` is incremented and then tested. If the promise and future are racing, they can both see `detached_ == 2` in the conditional, and then they'll both try to free the Core object. This fixes that.

It also fixes a related problem (which actually showed up more often with the test I wrote), where we transition into the Done state before setting the value, and then `maybeCallback()` observes the state is Done (because we're just reading an atomic, not grabbing the lock, which is intentional), tries to execute the callback, but `folly::Optional` throws an exception because the value hasn't been set yet (at least in debug it does). I should have listened to my gut and kept the state assignment after the transition action in the first place.

Test Plan: New unit test

Reviewed By: jsedgwick@fb.com

Subscribers: trunkagent, net-systems@, fugalh, exa, njormrod, folly-diffs@, mnd

FB internal diff: D1636490

Tasks: 5438209

Blame Revision: D1618240
folly/wangle/detail/Core.h
folly/wangle/detail/FSM.h
folly/wangle/test/FSM.cpp
folly/wangle/test/FutureTest.cpp