fix(rust): release ManagedStatement once via Arc inner Drop - #36
Open
fornwall wants to merge 1 commit into
Open
Conversation
ManagedStatement is Clone and wraps an Arc<ManagedStatementInner>, but the Drop impl lived on ManagedStatement itself, calling the driver's StatementRelease on every drop. Cloning then dropping the handle released the shared FFI statement more than once (double-free) and left surviving clones operating on a released statement (use-after-release). Move Drop onto ManagedStatementInner, the Arc-wrapped inner struct, so StatementRelease runs exactly once when the last Arc drops. This mirrors how ManagedConnectionInner and ManagedDatabaseInner already handle release. The driver is reached through the connection chain (self.connection.database.driver.driver). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNCrC87g9MkppGpL4MDgh5
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
1 participant
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.
The bug
ManagedStatementderivesCloneand wraps anArc<ManagedStatementInner>, but theDropimpl lived onManagedStatementitself and called the driver'sStatementReleaseon every drop. Because the handle isClone+Arc, cloning and then dropping released the shared FFI statement more than once — a double-free across the C ABI — and left any surviving clones operating on an already-released statement (use-after-release).The fix
Move the
Dropimpl fromManagedStatementonto the Arc-wrapped inner structManagedStatementInner, soStatementReleaseruns exactly once when the lastArcis dropped. This mirrors the existing, correct pattern used byManagedConnectionInnerandManagedDatabaseInner. The driver is reached through the connection chain (self.connection.database.driver.driver), and the existing TODO about handlingStatementReleasefailure is preserved.Verification
cargo build -p adbc_driver_manager— succeedscargo test -p adbc_driver_manager --lib— 18 passed, 0 failed, 13 ignored🤖 Generated with Claude Code
https://claude.ai/code/session_01XNCrC87g9MkppGpL4MDgh5