GH-48741: [C++] Fix deadlock in CSV AsyncThreadedTableReader destructor #48742

pitrou · 2026-01-06T16:38:26Z

Rationale for this change

When reading a CSV file encounters an early error, and the AsyncThreadedTableReader is only kept alive through the chain of futures and their callbacks, the ~AsyncThreadedTableReader destructor will be executed at the end of a TaskGroup task and try to wait for the TaskGroup itself for finish. This will obviously deadlock.

This issue was discovered by OSS-Fuzz in https://issues.oss-fuzz.com/issues/467451924

What changes are included in this PR?

Instead of waiting for the TaskGroup to finish in the AsyncThreadedTableReader, make sure that all async callbacks involved in CSV reading own their captured variables, to avoid use-after-free problems.

This has the side effect of keeping the AsyncThreadedTableReader alive until all relevant async callbacks have executed.

Are these changes tested?

Yes, by existing tests and a new fuzz regression test.

Are there any user-facing changes?

No.

GitHub Issue: [C++] Deadlock in CSV AsyncThreadedTableReader destructor #48741

pitrou · 2026-01-06T17:36:24Z

@github-actions crossbow submit -g cpp

zanmato1984 · 2026-01-07T02:25:42Z

Why is the deadlock only happening when early error? Also, is it possible to construct a dedicated test case?

pitrou · 2026-01-07T08:39:31Z

Why is the deadlock only happening when early error?

Because otherwise the task group is already finished (FinishAsync has finished) when the Future is marked successful.

Also, is it possible to construct a dedicated test case?

Hmm, that should hopefully be possible.

zanmato1984 · 2026-01-07T10:05:02Z

Why is the deadlock only happening when early error?

Because otherwise the task group is already finished (FinishAsync has finished) when the Future is marked successful.

I see, thanks.

Also, is it possible to construct a dedicated test case?

Hmm, that should hopefully be possible.

Yeah, let's have one please.

zanmato1984

The fix LGTM, only that a test might be needed.

pitrou · 2026-01-07T16:46:41Z

@github-actions crossbow submit -g cpp

pitrou · 2026-01-07T16:47:42Z

@zanmato1984 I added a test and also had to strengthen object ownership to avoid another regression, could you take another look?

github-actions · 2026-01-07T16:50:00Z

Revision: 12a8dea

Submitted crossbow builds: ursacomputing/crossbow @ actions-7019b48426

Task	Status
example-cpp-minimal-build-static
example-cpp-minimal-build-static-system-dependency
example-cpp-tutorial
test-build-cpp-fuzz
test-conda-cpp
test-conda-cpp-valgrind
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1
test-cuda-cpp-ubuntu-24.04-cuda-13.0.2
test-debian-12-cpp-amd64
test-debian-12-cpp-i386
test-debian-experimental-cpp-gcc-15
test-fedora-42-cpp
test-ubuntu-22.04-cpp
test-ubuntu-22.04-cpp-20
test-ubuntu-22.04-cpp-bundled
test-ubuntu-22.04-cpp-emscripten
test-ubuntu-22.04-cpp-no-threading
test-ubuntu-24.04-cpp
test-ubuntu-24.04-cpp-bundled-offline
test-ubuntu-24.04-cpp-gcc-13-bundled
test-ubuntu-24.04-cpp-gcc-14
test-ubuntu-24.04-cpp-minimal-with-formats
test-ubuntu-24.04-cpp-thread-sanitizer

zanmato1984 · 2026-01-08T03:46:47Z

cpp/src/arrow/csv/reader.cc

        return Status::OK();
      };

      return VisitAsyncGenerator(std::move(block_generator), block_visitor)


Sorry I have to double check the deadlock reasoning.

Seems that self is well captured by the returned future. So the deadlock happens only when: 1) early error happens and FinishAsync() is skipped; 2) the returned future is marked finished (by error) and released in its owning thread (the "main" thread), self ref count decreased but not to 0 (because of); 3) some task in the task group is still running, and the task holds a strong ref to self captured in block_visitor (so far the task group, tasks, and the self it owned are fully detached from the main thread?); 4) as the remaining tasks finish, self ref count goes to 0 and invokes the AsyncThreadedTableReader dtor, which in turn drains the task group in which the dtor is being invoked - deadlock?

If my understanding is correct, it doesn't seem that we have ownership issues:

If no error, then the main thread owns the future, which owns self, until all tasks are finished.

If early error, then the tasks own self.

self owns all the things: options, task group.

as the remaining tasks finish, self ref count goes to 0 and invokes the AsyncThreadedTableReader dtor, which in turn drains the task group in which the dtor is being invoked - deadlock?

Exactly.

If my understanding is correct, it doesn't seem that we have ownership issues:

If no error, then the main thread owns the future, which owns self, until all tasks are finished.

If early error, then the tasks own self.

self owns all the things: options, task group.

Please also see my this question :)

Hmm, what is the this question? I'm probably dense, but I can't find anywhere in the comments page :-S

Oh, don't know what's going on. Let me ask this: Given that my assumption of the cause of deadlock is correct, it doesn't seem to me that we have ownership issues:

If no error, then the main thread owns the future, which owns self, until all tasks are finished.

If early error, then the tasks own self.

In either case, self owns all the things: options, task group.

So why do we have to make all the ownership related changes in the rest of the PR?

Sorry, I may have an answer to my last question.

If early error, then the tasks own self.

self will be destructed in the last task, by when the task_group must be still alive. This explains the changes to the task_group_ ownership.

But why the changes to the options_ ownership? It doesn't have to out-live self, does it?

But why the changes to the options_ ownership? It doesn't have to out-live self, does it?

It does, because the ColumnBuilder can schedule its own tasks without owning the table reader:

arrow/cpp/src/arrow/csv/column_builder.cc

Lines 259 to 261 in fe5e0e5

void InferringColumnBuilder::ScheduleConvertChunk(int64_t chunk_index) {

task_group_->Append([this, chunk_index]() { return TryConvertChunk(chunk_index); });

}

Ah ok. Thank you.

zanmato1984

+1 with some nits.

cpp/src/arrow/csv/column_builder.cc

zanmato1984 · 2026-01-08T15:14:31Z

cpp/src/arrow/csv/reader.cc

 protected:
  // Make column builders from conversion schema
  Status MakeColumnBuilders() {
+    // This is making a single copy of ConvertOptions, which is reasonable even if


So this is n shared_ptr VS. n copies of ConvertOptions? (Which makes sense.)

zanmato1984 · 2026-01-08T15:14:50Z

cpp/src/arrow/csv/reader.cc

        return Status::OK();
      };

      return VisitAsyncGenerator(std::move(block_generator), block_visitor)


Ah ok. Thank you.

…structor

pitrou · 2026-01-08T17:17:51Z

I'm going to merge as CI is green. Thank you for the reviews @zanmato1984 !

conbench-apache-arrow · 2026-01-09T00:33:50Z

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

There weren't enough matching historic benchmark results to make a call on whether there were regressions.

The full Conbench report has more details.

pitrou marked this pull request as ready for review January 6, 2026 16:38

github-actions bot added Component: C++ awaiting review Awaiting review labels Jan 6, 2026

pitrou requested a review from zanmato1984 January 6, 2026 16:41

pitrou force-pushed the gh48741-csv-async-deadlock branch from 932821c to cdf530b Compare January 6, 2026 17:36

This comment was marked as outdated.

Sign in to view

zanmato1984 reviewed Jan 7, 2026

View reviewed changes

github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jan 7, 2026

pitrou force-pushed the gh48741-csv-async-deadlock branch 2 times, most recently from a8c352d to 12a8dea Compare January 7, 2026 16:46

zanmato1984 reviewed Jan 8, 2026

View reviewed changes

pitrou force-pushed the gh48741-csv-async-deadlock branch from 12a8dea to 530942d Compare January 8, 2026 15:01

zanmato1984 approved these changes Jan 8, 2026

View reviewed changes

apacheGH-48741: [C++] Fix deadlock in CSV AsyncThreadedTableReader de…

87316ce

…structor

pitrou force-pushed the gh48741-csv-async-deadlock branch from 530942d to 87316ce Compare January 8, 2026 15:37

pitrou merged commit 4eccdad into apache:main Jan 8, 2026
54 of 56 checks passed

pitrou removed the awaiting committer review Awaiting committer review label Jan 8, 2026

pitrou mentioned this pull request Jan 8, 2026

[C++] Deadlock in CSV AsyncThreadedTableReader destructor #48741

Closed

pitrou deleted the gh48741-csv-async-deadlock branch January 8, 2026 17:18

	void InferringColumnBuilder::ScheduleConvertChunk(int64_t chunk_index) {
	task_group_->Append([this, chunk_index]() { return TryConvertChunk(chunk_index); });
	}

GH-48741: [C++] Fix deadlock in CSV AsyncThreadedTableReader destructor #48742

GH-48741: [C++] Fix deadlock in CSV AsyncThreadedTableReader destructor #48742

Conversation

pitrou commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

pitrou commented Jan 6, 2026

Uh oh!

This comment was marked as outdated.

zanmato1984 commented Jan 7, 2026

Uh oh!

pitrou commented Jan 7, 2026

Uh oh!

zanmato1984 commented Jan 7, 2026

Uh oh!

zanmato1984 left a comment

Choose a reason for hiding this comment

Uh oh!

pitrou commented Jan 7, 2026

Uh oh!

pitrou commented Jan 7, 2026

Uh oh!

github-actions bot commented Jan 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zanmato1984 Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zanmato1984 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pitrou commented Jan 8, 2026

Uh oh!

Uh oh!

conbench-apache-arrow bot commented Jan 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pitrou commented Jan 6, 2026 •

edited

Loading

zanmato1984 Jan 8, 2026 •

edited

Loading