Repository navigation
Conversation
…ct (tursodatabase#2295) SQLite's sqlite3_update_hook retains a pointer to the callback context without taking a deallocation callback. In libsql, add_update_hook boxes the closure in a Container and leaks it to FFI. When disconnect was called, sqlite3_close_v2 was invoked without deregistering the hook, permanently leaking any captured Arcs and closure state. This commit deregisters the hook in Connection::disconnect before closing, reclaiming the Container via Box::from_raw. Also adds an integration test verifying captures are deallocated on connection drop, and gates pprof to non-windows targets in dev-dependencies.
This branch has not been deployed
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.
Problem
As reported in #2295, registering an update hook through
Connection::add_update_hookleaks the final callback allocation (Box<Container>) and all values captured by its closure when the local connection closes.SQLite's
sqlite3_update_hookC API has no built-in destructor callback. Inlibsql,add_update_hookboxes the closure into aContainerand passes a leaked pointer viaBox::leak. While subsequent calls toadd_update_hookcorrectly reclaim the previous context,Connection::disconnectwas previously invokingsqlite3_close_v2without first unsetting the hook, permanently leaking the final context allocation and any capturedArcpointers, channels, or resources.Solution
Connection::disconnect, deregister the hook prior to callingsqlite3_close_v2by callingffi::sqlite3_update_hook(self.raw, None, std::ptr::null_mut()). If a context pointer was returned, reclaim and deallocate it viaBox::from_raw(old_data as *mut Container).update_hook_closure_dropped_on_disconnect) verifying that closure captures (Arc) are deallocated when the connection is dropped.pprofdev-dependency to non-windows targets ([target.'cfg(not(windows))'.dev-dependencies]) inlibsql/Cargo.tomlso test suites compile cleanly across platforms.Fixes #2295.