fix: add ScopeGuard to wait async tasks before early return in OrphanFilesCleanerImpl::Clean()#307
Merged
Merged
Conversation
zjw1111
force-pushed
the
fix/orphan-cleaner-segfault
branch
from
8386f5d to
36e42de
Compare
zjw1111
force-pushed
the
fix/orphan-cleaner-segfault
branch
2 times, most recently
from
977a477 to
ee6d26d
Compare
zjw1111
force-pushed
the
fix/orphan-cleaner-segfault
branch
from
032b8b0 to
7e5aae1
Compare
Collaborator
Author
|
@Eyizoha, the exception handling logic ( |
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.
zjw1111
force-pushed
the
fix/orphan-cleaner-segfault
branch
from
6b345a4 to
0e54c34
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Linked issue: N/A
Fix intermittent segmentation fault in
OrphanFilesCleanerImpl::Clean(). WhenGetUsedFiles()returns an early error (e.g. "do not support clean index manifest"), the function returns immediately while thread pool tasks submitted viaVia(executor_.get(), ...)are still running. These tasks capturethisand access members likefs_, causing use-after-free when theOrphanFilesCleanerImplobject is destroyed shortly after.Changes:
ScopeGuardinClean()that callsCollectAll(file_statuses_futures)to ensure all submitted async tasks complete before the function returns on early error exit.future.valid()check inCollectAll()(infuture.h) to safely skip already-consumed futures, so the guard is a no-op on the normal path whereCollectAllis explicitly called later. This is consistent with the existingWait()function which already has the samevalid()check.Via()noting that the exception capture logic will be removed in the future since paimon-cpp uses Status/Result for error handling throughout.Tests
Existing test
OrphanFilesCleanerTest.TestTableWithIndexManifestcovers this path — it was the one experiencing the intermittent segfault.API and Format
No.
Documentation
No.
Generative AI tooling
Generated-by: Aone Copilot (Claude 4.6 Opus)