facebook / facebook/folly

CPUThreadPoolExecutor bug if thread creation fails

Open
#1,428 2 comments 1 reaction 0 assignees View on GitHub
Dominant language
C++
Stars
30.5k
Forks
5.9k
PR merge metrics
No merged PRs in 30d

Description

`CPUThreadPoolExecutor::add` does this:

```
auto result = taskQueue_->add(
CPUTask(std::move(func), expiration, std::move(expireCallback)));
if (!result.reusedThread) {
ensureActiveThreads();
}
```

That is, it adds the task to the queue, and then tries to create new threads if necessary and allowed.

If thread creation fails because of hitting a system limit, `add()` throws from `ensureActiveThreads()`, but the task was still added to the queue and so it will be executed.

This breaks `folly::via`:

```
auto f = folly::via(executor).thenValue(...);
```

`add` throwing makes `f` complete with an exception, but then, when the `CPUThreadPoolExecutor` task gets run, it tries to complete the future again, leading to "promise invalid" errors in release mode, and more fun assertions in debug mode:

```
ThreadPoolExecutor.cpp:91] ThreadPoolExecutor: func threw unhandled N5folly14PromiseInvalidE exception: Promise invalid
```

```
ft: /usr/local/include/folly/futures/Future-inl.h:142: auto folly::futures::detail::CoreCallbackState::invoke(Args&& ...) [with Args = {folly::Executor::KeepAlive, folly::Try}; T = folly::Unit; F = folly::Future::thenValue(F&&) && [with F = rockset::{anonymous}::run(int, char**)::; T = folly::Unit; typename folly::futures::detail::valueCallableResult::value_type = folly::Unit]::&&, folly::Try&&)>]: Assertion `before_barrier()' failed.
```

To reproduce, I forced thread creation failure by artificially limiting the process's address space and requiring each thread to use a lot of stack space:

```
folly::CPUThreadPoolExecutor executor(1000);

struct rlimit rl;
folly::checkUnixError(getrlimit(RLIMIT_AS, &rl), "getrlimit");

// 1GiB
rl.rlim_cur = rl.rlim_max = 1L << 30;
folly::checkUnixError(setrlimit(RLIMIT_AS, &rl), "setrlimit");

pthread_attr_t attr;
pthread_attr_init(&attr);
folly::checkPosixError(pthread_attr_setstacksize(&attr, 1UL << 27),
"pthread_attr_setstacksize");
folly::checkPosixError(pthread_setattr_default_np(&attr),
"pthread_setattr_default_np");

vector> fs;
for (int i = 0; i < 100; ++i) {
fs.push_back(folly::via(&executor).thenValue(
[&](folly::Unit) { std::this_thread::sleep_for(5s); }));
}

std::cout << "created ops\n";

auto rs = folly::collectAll(std::move(fs)).get();
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.