GH-47642: [C++] Catch exceptions from initial_task in AsyncTaskScheduler

[egolearner]

Rationale for this change

If the initial_task passed to AsyncTaskScheduler::Make throws a C++ exception (rather than returning a failed Status), OnTaskFinished is never called. This leaves running_tasks_ permanently at 1, causing a DCHECK failure in debug builds and an indefinite hang in release builds because the scheduler's finished future is never completed. In Acero, this manifests as DeclarationToTable (and similar APIs) hanging forever when a SourceNode generator throws during StartProducing.

What changes are included in this PR?

Add exception handling logic.

Are these changes tested?

Yes.

Are there any user-facing changes?

No API changes.

• GitHub Issue: [C++][Acero] If initial_task in Acero plan throws an exception the plan hangs indefinitely #47642

[github-actions]

⚠️ GitHub issue #47642 has been automatically assigned in GitHub to PR creator.

[pitrou]

If the initial_task passed to AsyncTaskScheduler::Make throws a C++ exception

Why would it throw at all? Arrow uses Status-based error reporting. If we were catching exceptions from any user-provided callable, this would litter the Arrow C++ code with ad hoc catch statements. We would also certainly forget to catch exceptions in some place, and users would regularly have to report issues about that.

For reference, our ThreadPool doesn't catch exceptions from user tasks either. They are not supposed to throw exceptions.

[egolearner]

If the initial_task passed to AsyncTaskScheduler::Make throws a C++ exception

Why would it throw at all? Arrow uses Status-based error reporting. If we were catching exceptions from any user-provided callable, this would litter the Arrow C++ code with ad hoc catch statements. We would also certainly forget to catch exceptions in some place, and users would regularly have to report issues about that.

For reference, our ThreadPool doesn't catch exceptions from user tasks either. They are not supposed to throw exceptions.

Thanks for the pushback, but I'd like to make a case for keeping the catch here.

The failure mode is asymmetric with the rest of Arrow: everywhere else a buggy user callable surfaces as a Status, but here it silently hangs forever in release builds (running_tasks_ stays at 1, the plan's finished future never completes). The ThreadPool comparison doesn't quite apply either — a throwing task there calls std::terminate, so the user gets a stack trace. Here they get silence.

Pushing the catch down to call sites (e.g. SourceNode::StartProducing) fixes the case I hit but is whack-a-mole — any future caller that hands a throwing callable to initial_task will hang again. The centralized fix is one try/catch on the scheduler's bootstrap path, not scattered through the codebase.

[pitrou]

The failure mode is asymmetric with the rest of Arrow: everywhere else a buggy user callable surfaces as a Status, but here it silently hangs forever in release builds (running_tasks_ stays at 1, the plan's finished future never completes). The ThreadPool comparison doesn't quite apply either — a throwing task there calls std::terminate, so the user gets a stack trace. Here they get silence.

Hmm, that's a good point.

[pitrou]

+1, thank you @egolearner

[github-actions]

⚠️ GitHub issue #47642 has been automatically assigned in GitHub to PR creator.

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit e2b4437.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.