Commit f4c8e833 authored by Yedidya Feldblum's avatar Yedidya Feldblum Committed by Facebook GitHub Bot

fix race handling bug in futures interrupt-handler

Summary:
Setting the interrupt-handler races with setting the interrupt-exception. This is expected to occur on occasion and should be handled in both `setInterruptHandler` and `raise`. The latter is correct but the former has a bug when, if the race happens, a moved-from interrupt-handler is invoked. The fix is to be sure to invoke a non-moved-from handler.

For consistency, always performs precisely one copy or move of the handler and always invokes the handler as an lvalue-ref-to-non-const.

Reviewed By: iahs

Differential Revision: D29974165

fbshipit-source-id: faf083e97c042fda0622801fc94bbc6fbca910fd
parent d0edf4c6
...@@ -436,11 +436,12 @@ class CoreBase { ...@@ -436,11 +436,12 @@ class CoreBase {
if (hasResult()) { if (hasResult()) {
return; return;
} }
handler_type* handler = nullptr;
auto interrupt = interrupt_.load(std::memory_order_acquire); auto interrupt = interrupt_.load(std::memory_order_acquire);
switch (interrupt & InterruptMask) { switch (interrupt & InterruptMask) {
case InterruptInitial: { // store the handler case InterruptInitial: { // store the handler
assert(!interrupt); assert(!interrupt);
auto handler = new handler_type(static_cast<F&&>(fn)); handler = new handler_type(static_cast<F&&>(fn));
auto exchanged = folly::atomic_compare_exchange_strong_explicit( auto exchanged = folly::atomic_compare_exchange_strong_explicit(
&interrupt_, &interrupt_,
&interrupt, &interrupt,
...@@ -451,7 +452,6 @@ class CoreBase { ...@@ -451,7 +452,6 @@ class CoreBase {
return; return;
} }
// lost the race! // lost the race!
delete handler;
if (interrupt & InterruptHasHandler) { if (interrupt & InterruptHasHandler) {
terminate_with<std::logic_error>("set-interrupt-handler race"); terminate_with<std::logic_error>("set-interrupt-handler race");
} }
...@@ -466,7 +466,14 @@ class CoreBase { ...@@ -466,7 +466,14 @@ class CoreBase {
} }
auto pointer = interrupt & ~InterruptMask; auto pointer = interrupt & ~InterruptMask;
auto object = reinterpret_cast<exception_wrapper*>(pointer); auto object = reinterpret_cast<exception_wrapper*>(pointer);
fn(as_const(*object)); if (handler) {
handler->handle(*object);
delete handler;
} else {
// mimic constructing and invoking a handler: 1 copy; non-const invoke
auto fn_ = static_cast<F&&>(fn);
fn_(as_const(*object));
}
delete object; delete object;
return; return;
} }
......
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