Skip to content

Commit e2b4437

Browse files
egolearnerpitrou
andauthored
GH-47642: [C++] Catch exceptions from initial_task in AsyncTaskScheduler (#49860)
### 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: #47642 Lead-authored-by: egolearner <lijiliang@outlook.com> Co-authored-by: Antoine Pitrou <pitrou@free.fr> Signed-off-by: Antoine Pitrou <antoine@python.org>
1 parent 35eed28 commit e2b4437

2 files changed

Lines changed: 41 additions & 1 deletion

File tree

cpp/src/arrow/util/async_util.cc

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "arrow/util/tracing_internal.h"
2424

2525
#include <condition_variable>
26+
#include <exception>
2627
#include <list>
2728
#include <memory>
2829
#include <mutex>
@@ -466,7 +467,19 @@ Future<> AsyncTaskScheduler::Make(FnOnce<Status(AsyncTaskScheduler*)> initial_ta
466467
auto scope = START_SCOPED_SPAN_SV(span, "AsyncTaskScheduler::InitialTask"sv);
467468
auto scheduler = std::make_unique<AsyncTaskSchedulerImpl>(std::move(stop_token),
468469
std::move(abort_callback));
469-
Status initial_task_st = std::move(initial_task)(scheduler.get());
470+
Status initial_task_st;
471+
// GH-47642: We normally don't catch exceptions in Arrow C++ code, as the error
472+
// reporting model uses the Status object instead. Usually, an uncaught exception
473+
// will simply terminate the process, surfacing the programming error.
474+
// However, an exception thrown from the initial task would result in a much
475+
// harder to diagnose process hang.
476+
try {
477+
initial_task_st = std::move(initial_task)(scheduler.get());
478+
} catch (const std::exception& e) {
479+
initial_task_st = Status::UnknownError("Initial task threw an exception: ", e.what());
480+
} catch (...) {
481+
initial_task_st = Status::UnknownError("Initial task threw an unknown exception");
482+
}
470483
scheduler->OnTaskFinished(std::move(initial_task_st));
471484
// Keep scheduler alive until finished
472485
return scheduler->OnFinished().Then([scheduler = std::move(scheduler)] {});

cpp/src/arrow/util/async_util_test.cc

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@
2323
#include <memory>
2424
#include <mutex>
2525
#include <queue>
26+
#include <stdexcept>
2627
#include <thread>
2728
#include <unordered_set>
2829

30+
#include <gmock/gmock-matchers.h>
2931
#include <gtest/gtest.h>
3032

3133
#include "arrow/result.h"
@@ -204,6 +206,31 @@ TEST(AsyncTaskScheduler, InitialTaskFails) {
204206
ASSERT_FINISHES_AND_RAISES(Invalid, finished);
205207
}
206208

209+
TEST(AsyncTaskScheduler, InitialTaskThrowsException) {
210+
// If the initial task throws a C++ exception (not a Status), the scheduler
211+
// should catch it, convert to a failed Status, and not hang indefinitely.
212+
// See https://github.com/apache/arrow/issues/47642
213+
214+
// Case 1: initial task throws with no other tasks
215+
Future<> finished =
216+
AsyncTaskScheduler::Make([&](AsyncTaskScheduler* scheduler) -> Status {
217+
throw std::runtime_error("some exception");
218+
});
219+
EXPECT_FINISHES_AND_RAISES_WITH_MESSAGE_THAT(
220+
UnknownError, ::testing::HasSubstr("some exception"), finished);
221+
222+
// Case 2: initial task throws while another task is still running
223+
Future<> task = Future<>::Make();
224+
finished = AsyncTaskScheduler::Make([&](AsyncTaskScheduler* scheduler) -> Status {
225+
EXPECT_TRUE(scheduler->AddSimpleTask([&]() { return task; }, kDummyName));
226+
throw std::runtime_error("some exception after adding task");
227+
});
228+
AssertNotFinished(finished);
229+
task.MarkFinished();
230+
EXPECT_FINISHES_AND_RAISES_WITH_MESSAGE_THAT(
231+
UnknownError, ::testing::HasSubstr("some exception after adding task"), finished);
232+
}
233+
207234
TEST(AsyncTaskScheduler, TaskDestroyedBeforeSchedulerEnds) {
208235
bool my_task_destroyed = false;
209236
Future<> task_fut = Future<>::Make();

0 commit comments

Comments
 (0)