From 079e4ffe2f7d109c68bb66d7455d90724d7047fc Mon Sep 17 00:00:00 2001 From: Matt Dordal Date: Wed, 17 Sep 2014 16:52:03 -0700 Subject: [PATCH] fix race in Future::waitWithSemaphore Summary: There's a race condition in waitWithSemaphore, specifically because we're returning a new future that's completed by the input future. As a result, that future may not be completed by the time waitWithSemaphore returns, although completion should be imminent. Test Plan: `var=0; while true ; do echo $((var++)); _bin/folly/wangle/wangle-test --gtest_filter='Future*' || break ; done` Before change two runs yielded 303 and 371. After change I killed the test at 11000. Reviewed By: njormrod@fb.com Subscribers: fugalh, njormrod FB internal diff: D1561281 Tasks: 5180879 --- folly/wangle/Future-inl.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/folly/wangle/Future-inl.h b/folly/wangle/Future-inl.h index de8dbf4bdb6..f8751f5ef8b 100644 --- a/folly/wangle/Future-inl.h +++ b/folly/wangle/Future-inl.h @@ -17,6 +17,7 @@ #pragma once #include +#include #include #include @@ -416,6 +417,12 @@ waitWithSemaphore(Future&& f) { return std::move(t.value()); }); baton.wait(); + while (!done.isReady()) { + // There's a race here between the return here and the actual finishing of + // the future. f is completed, but the setup may not have finished on done + // after the baton has posted. + std::this_thread::yield(); + } return done; } @@ -427,6 +434,12 @@ inline Future waitWithSemaphore(Future&& f) { t.value(); }); baton.wait(); + while (!done.isReady()) { + // There's a race here between the return here and the actual finishing of + // the future. f is completed, but the setup may not have finished on done + // after the baton has posted. + std::this_thread::yield(); + } return done; }