Commit d28b94ab authored by Andrii Grynenko's avatar Andrii Grynenko Committed by Facebook Github Bot

Disable future unwrapping in SemiFuture::defer(Value/Error)

Summary: Future unwrapping in defer is broken for incomplete Futures and SemiFutures with deferred work.

Reviewed By: yfeldblum

Differential Revision: D8675535

fbshipit-source-id: 8deff362ea018cf4717354cd973756161c4249aa
parent 860b718e
...@@ -540,6 +540,7 @@ class DeferredExecutor final : public Executor { ...@@ -540,6 +540,7 @@ class DeferredExecutor final : public Executor {
} }
void setExecutor(folly::Executor* executor) { void setExecutor(folly::Executor* executor) {
DCHECK(!dynamic_cast<DeferredExecutor*>(executor));
if (nestedExecutors_) { if (nestedExecutors_) {
for (auto nestedExecutor : *nestedExecutors_) { for (auto nestedExecutor : *nestedExecutors_) {
nestedExecutor->setExecutor(executor); nestedExecutor->setExecutor(executor);
...@@ -880,6 +881,9 @@ template <class T> ...@@ -880,6 +881,9 @@ template <class T>
template <typename F> template <typename F>
SemiFuture<typename futures::detail::tryCallableResult<T, F>::value_type> SemiFuture<typename futures::detail::tryCallableResult<T, F>::value_type>
SemiFuture<T>::defer(F&& func) && { SemiFuture<T>::defer(F&& func) && {
static_assert(
!futures::detail::tryCallableResult<T, F>::ReturnsFuture::value,
"defer does not support Future unwrapping");
DeferredExecutor* deferredExecutor = getDeferredExecutor(); DeferredExecutor* deferredExecutor = getDeferredExecutor();
if (!deferredExecutor) { if (!deferredExecutor) {
deferredExecutor = new DeferredExecutor(); deferredExecutor = new DeferredExecutor();
...@@ -898,6 +902,9 @@ template <class T> ...@@ -898,6 +902,9 @@ template <class T>
template <typename F> template <typename F>
SemiFuture<typename futures::detail::valueCallableResult<T, F>::value_type> SemiFuture<typename futures::detail::valueCallableResult<T, F>::value_type>
SemiFuture<T>::deferValue(F&& func) && { SemiFuture<T>::deferValue(F&& func) && {
static_assert(
!futures::detail::valueCallableResult<T, F>::ReturnsFuture::value,
"deferValue does not support Future unwrapping");
return std::move(*this).defer([f = std::forward<F>(func)]( return std::move(*this).defer([f = std::forward<F>(func)](
folly::Try<T>&& t) mutable { folly::Try<T>&& t) mutable {
return std::forward<F>(f)( return std::forward<F>(f)(
...@@ -910,13 +917,17 @@ SemiFuture<T>::deferValue(F&& func) && { ...@@ -910,13 +917,17 @@ SemiFuture<T>::deferValue(F&& func) && {
template <class T> template <class T>
template <class ExceptionType, class F> template <class ExceptionType, class F>
SemiFuture<T> SemiFuture<T>::deferError(F&& func) && { SemiFuture<T> SemiFuture<T>::deferError(F&& func) && {
static_assert(
!isFutureOrSemiFuture<decltype(
std::forward<F>(func)(std::declval<ExceptionType&>()))>::value,
"deferError does not support Future unwrapping");
return std::move(*this).defer( return std::move(*this).defer(
[func = std::forward<F>(func)](Try<T>&& t) mutable { [func = std::forward<F>(func)](Try<T>&& t) mutable {
if (auto e = t.template tryGetExceptionObject<ExceptionType>()) { if (auto e = t.template tryGetExceptionObject<ExceptionType>()) {
return makeSemiFutureWith( return Try<T>(makeTryWith([&] { return std::forward<F>(func)(*e); }))
[&]() mutable { return std::forward<F>(func)(*e); }); .value();
} else { } else {
return makeSemiFuture<T>(std::move(t)); return std::move(*t);
} }
}); });
} }
...@@ -924,14 +935,19 @@ SemiFuture<T> SemiFuture<T>::deferError(F&& func) && { ...@@ -924,14 +935,19 @@ SemiFuture<T> SemiFuture<T>::deferError(F&& func) && {
template <class T> template <class T>
template <class F> template <class F>
SemiFuture<T> SemiFuture<T>::deferError(F&& func) && { SemiFuture<T> SemiFuture<T>::deferError(F&& func) && {
static_assert(
!isFutureOrSemiFuture<decltype(
std::forward<F>(func)(std::declval<exception_wrapper&>()))>::value,
"deferError does not support Future unwrapping");
return std::move(*this).defer( return std::move(*this).defer(
[func = std::forward<F>(func)](Try<T> t) mutable { [func = std::forward<F>(func)](Try<T> t) mutable {
if (t.hasException()) { if (t.hasException()) {
return makeSemiFutureWith([&]() mutable { return Try<T>(makeTryWith([&] {
return std::forward<F>(func)(std::move(t.exception())); return std::forward<F>(func)(std::move(t.exception()));
}); }))
.value();
} else { } else {
return makeSemiFuture<T>(std::move(t)); return std::move(*t);
} }
}); });
} }
...@@ -942,7 +958,7 @@ SemiFuture<T> SemiFuture<T>::delayed(Duration dur, Timekeeper* tk) && { ...@@ -942,7 +958,7 @@ SemiFuture<T> SemiFuture<T>::delayed(Duration dur, Timekeeper* tk) && {
.toUnsafeFuture() .toUnsafeFuture()
.then([](std::tuple<Try<T>, Try<Unit>> tup) { .then([](std::tuple<Try<T>, Try<Unit>> tup) {
Try<T>& t = std::get<0>(tup); Try<T>& t = std::get<0>(tup);
return makeFuture<T>(std::move(t)); return makeFuture<T>(std::move(*t));
}); });
} }
......
...@@ -255,11 +255,11 @@ TEST(SemiFuture, hasPostconditionInvalid) { ...@@ -255,11 +255,11 @@ TEST(SemiFuture, hasPostconditionInvalid) {
} }
namespace { namespace {
SemiFuture<int> onErrorHelperEggs(const eggs_t&) { int onErrorHelperEggs(const eggs_t&) {
return makeSemiFuture(10); return 10;
} }
SemiFuture<int> onErrorHelperGeneric(const folly::exception_wrapper&) { int onErrorHelperGeneric(const folly::exception_wrapper&) {
return makeSemiFuture(20); return 20;
} }
} // namespace } // namespace
...@@ -800,17 +800,6 @@ TEST(SemiFuture, onError) { ...@@ -800,17 +800,6 @@ TEST(SemiFuture, onError) {
EXPECT_FLAG(); EXPECT_FLAG();
} }
{
auto f = makeSemiFuture()
.defer([] { throw eggs; })
.deferError<eggs_t>([&](eggs_t const& /* e */) {
flag();
return makeSemiFuture();
});
EXPECT_NO_THROW(std::move(f).get());
EXPECT_FLAG();
}
// By auto reference // By auto reference
{ {
auto f = makeSemiFuture() auto f = makeSemiFuture()
...@@ -820,17 +809,6 @@ TEST(SemiFuture, onError) { ...@@ -820,17 +809,6 @@ TEST(SemiFuture, onError) {
EXPECT_FLAG(); EXPECT_FLAG();
} }
{
auto f = makeSemiFuture()
.defer([] { throw eggs; })
.deferError<eggs_t>([&](auto& /* e */) {
flag();
return makeSemiFuture();
});
EXPECT_NO_THROW(std::move(f).get());
EXPECT_FLAG();
}
// By value // By value
{ {
auto f = makeSemiFuture() auto f = makeSemiFuture()
...@@ -840,25 +818,11 @@ TEST(SemiFuture, onError) { ...@@ -840,25 +818,11 @@ TEST(SemiFuture, onError) {
EXPECT_FLAG(); EXPECT_FLAG();
} }
{
auto f = makeSemiFuture()
.defer([] { throw eggs; })
.deferError<eggs_t>([&](eggs_t /* e */) {
flag();
return makeSemiFuture();
});
EXPECT_NO_THROW(std::move(f).get());
EXPECT_FLAG();
}
// auto value // auto value
{ {
auto f = makeSemiFuture() auto f = makeSemiFuture()
.defer([] { throw eggs; }) .defer([] { throw eggs; })
.deferError<eggs_t>([&](auto /* e */) { .deferError<eggs_t>([&](auto /* e */) { flag(); });
flag();
return makeSemiFuture();
});
EXPECT_NO_THROW(std::move(f).get()); EXPECT_NO_THROW(std::move(f).get());
EXPECT_FLAG(); EXPECT_FLAG();
} }
...@@ -873,17 +837,6 @@ TEST(SemiFuture, onError) { ...@@ -873,17 +837,6 @@ TEST(SemiFuture, onError) {
EXPECT_FLAG(); EXPECT_FLAG();
} }
{
auto f = makeSemiFuture()
.defer([] { throw eggs; })
.deferError<std::exception>([&](auto const& /* e */) {
flag();
return makeSemiFuture();
});
EXPECT_NO_THROW(std::move(f).get());
EXPECT_FLAG();
}
// Non-exceptions // Non-exceptions
{ {
auto f = makeSemiFuture() auto f = makeSemiFuture()
...@@ -893,17 +846,6 @@ TEST(SemiFuture, onError) { ...@@ -893,17 +846,6 @@ TEST(SemiFuture, onError) {
EXPECT_FLAG(); EXPECT_FLAG();
} }
{
auto f = makeSemiFuture()
.defer([] { throw - 1; })
.deferError<int>([&](auto /* e */) {
flag();
return makeSemiFuture();
});
EXPECT_NO_THROW(std::move(f).get());
EXPECT_FLAG();
}
// Mutable lambda // Mutable lambda
{ {
auto f = auto f =
...@@ -914,17 +856,6 @@ TEST(SemiFuture, onError) { ...@@ -914,17 +856,6 @@ TEST(SemiFuture, onError) {
EXPECT_FLAG(); EXPECT_FLAG();
} }
{
auto f = makeSemiFuture()
.defer([] { throw eggs; })
.deferError<eggs_t>([&](auto const& /* e */) mutable {
flag();
return makeSemiFuture();
});
EXPECT_NO_THROW(std::move(f).get());
EXPECT_FLAG();
}
// Function pointer // Function pointer
{ {
auto f = makeSemiFuture() auto f = makeSemiFuture()
...@@ -960,17 +891,6 @@ TEST(SemiFuture, onError) { ...@@ -960,17 +891,6 @@ TEST(SemiFuture, onError) {
EXPECT_NO_FLAG(); EXPECT_NO_FLAG();
} }
{
auto f = makeSemiFuture()
.defer([] { return 42; })
.deferError<eggs_t>([&](auto const& /* e */) {
flag();
return makeSemiFuture<int>(-1);
});
EXPECT_EQ(42, std::move(f).get());
EXPECT_NO_FLAG();
}
// Catch different exception // Catch different exception
{ {
auto f = makeSemiFuture() auto f = makeSemiFuture()
...@@ -981,17 +901,6 @@ TEST(SemiFuture, onError) { ...@@ -981,17 +901,6 @@ TEST(SemiFuture, onError) {
EXPECT_NO_FLAG(); EXPECT_NO_FLAG();
} }
{
auto f = makeSemiFuture()
.defer([] { throw eggs; })
.deferError<std::runtime_error>([&](auto const& /* e */) {
flag();
return makeSemiFuture();
});
EXPECT_THROW(std::move(f).get(), eggs_t);
EXPECT_NO_FLAG();
}
// Returned value propagates // Returned value propagates
{ {
auto f = makeSemiFuture() auto f = makeSemiFuture()
...@@ -1000,16 +909,6 @@ TEST(SemiFuture, onError) { ...@@ -1000,16 +909,6 @@ TEST(SemiFuture, onError) {
EXPECT_EQ(42, std::move(f).get()); EXPECT_EQ(42, std::move(f).get());
} }
// Returned future propagates
{
auto f = makeSemiFuture()
.defer([]() -> int { throw eggs; })
.deferError<eggs_t>([&](auto const& /* e */) {
return makeSemiFuture<int>(42);
});
EXPECT_EQ(42, std::move(f).get());
}
// Throw in callback // Throw in callback
{ {
auto f = makeSemiFuture() auto f = makeSemiFuture()
...@@ -1018,38 +917,6 @@ TEST(SemiFuture, onError) { ...@@ -1018,38 +917,6 @@ TEST(SemiFuture, onError) {
EXPECT_THROW(std::move(f).get(), eggs_t); EXPECT_THROW(std::move(f).get(), eggs_t);
} }
{
auto f = makeSemiFuture()
.defer([]() -> int { throw eggs; })
.deferError<eggs_t>(
[&](auto const& e) -> SemiFuture<int> { throw e; });
EXPECT_THROW(std::move(f).get(), eggs_t);
}
// exception_wrapper, return Future<T>
{
auto f = makeSemiFuture()
.defer([] { throw eggs; })
.deferError([&](exception_wrapper /* e */) {
flag();
return makeSemiFuture();
});
EXPECT_NO_THROW(std::move(f).get());
EXPECT_FLAG();
}
// exception_wrapper, return Future<T> but throw
{
auto f = makeSemiFuture()
.defer([]() -> int { throw eggs; })
.deferError([&](exception_wrapper /* e */) -> SemiFuture<int> {
flag();
throw eggs;
});
EXPECT_THROW(std::move(f).get(), eggs_t);
EXPECT_FLAG();
}
// exception_wrapper, return T // exception_wrapper, return T
{ {
auto f = makeSemiFuture() auto f = makeSemiFuture()
...@@ -1080,7 +947,6 @@ TEST(SemiFuture, onError) { ...@@ -1080,7 +947,6 @@ TEST(SemiFuture, onError) {
.defer([] { throw eggs; }) .defer([] { throw eggs; })
.deferError([&](const exception_wrapper& /* e */) { .deferError([&](const exception_wrapper& /* e */) {
flag(); flag();
return makeSemiFuture();
}); });
EXPECT_NO_THROW(std::move(f).get()); EXPECT_NO_THROW(std::move(f).get());
EXPECT_FLAG(); EXPECT_FLAG();
...@@ -1094,15 +960,15 @@ TEST(SemiFuture, makePromiseContract) { ...@@ -1094,15 +960,15 @@ TEST(SemiFuture, makePromiseContract) {
EXPECT_EQ(4, std::move(c.second).get()); EXPECT_EQ(4, std::move(c.second).get());
} }
TEST(SemiFuture, invokeCallbackReturningFutureWithOriginalCVRef) { TEST(SemiFuture, invokeCallbackWithOriginalCVRef) {
struct Foo { struct Foo {
Future<int> operator()(int x) & { int operator()(int x) & {
return x + 1; return x + 1;
} }
Future<int> operator()(int x) const& { int operator()(int x) const& {
return x + 2; return x + 2;
} }
Future<int> operator()(int x) && { int operator()(int x) && {
return x + 3; return x + 3;
} }
}; };
......
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