Skip to content

Commit 7e5aae1

Browse files
committed
fix: add ScopeGuard to wait async tasks before early return in Clean()
When GetUsedFiles() returns an error (e.g. index manifest not supported), Clean() would return immediately while thread pool tasks submitted via Via(executor_.get(), ...) were still running. These tasks capture 'this' and access members like fs_, causing use-after-free when the OrphanFilesCleanerImpl is destroyed shortly after. Add a ScopeGuard that calls CollectAll(file_statuses_futures) to ensure all submitted async tasks complete before the function returns, preventing the intermittent segmentation fault.
1 parent 2d5fdbf commit 7e5aae1

2 files changed

Lines changed: 7 additions & 1 deletion

File tree

src/paimon/common/executor/future.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ namespace paimon {
4545
/// execution.
4646
///
4747
/// @note If `func` returns `void`, the returned future is of type `std::future<void>`.
48+
///
49+
/// TODO: Since paimon-cpp uses `Status`/`Result` for error handling throughout, the exception
50+
/// capture logic (try/catch + set_exception) in `Via()` will be removed in the future.
4851
template <typename Func>
4952
auto Via(Executor* executor, Func&& func) -> std::future<decltype(func())> {
5053
using ResultType = decltype(func());
@@ -96,7 +99,7 @@ std::vector<T> CollectAll(std::vector<std::future<T>>& futures) {
9699
std::vector<T> results;
97100
results.reserve(futures.size()); // Reserve space to avoid reallocation.
98101
for (auto& future : futures) {
99-
results.push_back(future.get()); // Wait for each future and collect the result.
102+
results.push_back(future.get());
100103
}
101104

102105
return results;

src/paimon/core/operation/orphan_files_cleaner_impl.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,14 @@ Result<std::set<std::string>> OrphanFilesCleanerImpl::Clean() {
9797
}
9898
PAIMON_ASSIGN_OR_RAISE(std::set<std::string> all_dirs, ListPaimonFileDirs());
9999
std::vector<std::future<std::vector<std::unique_ptr<FileStatus>>>> file_statuses_futures;
100+
ScopeGuard file_statuses_guard(
101+
[&file_statuses_futures]() { CollectAll(file_statuses_futures); });
100102
for (const auto& dir : all_dirs) {
101103
file_statuses_futures.push_back(
102104
Via(executor_.get(), [this, dir] { return TryBestListingDirs(dir); }));
103105
}
104106
PAIMON_ASSIGN_OR_RAISE(std::set<std::string> used_file_names, GetUsedFiles());
107+
file_statuses_guard.Release();
105108

106109
Duration duration;
107110
std::set<std::string> need_to_deletes;

0 commit comments

Comments
 (0)