-
Notifications
You must be signed in to change notification settings - Fork 230
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add sqlite3_close_hook #219
Conversation
The callback allows registering a custom close hook, which gets called prior to closing the connection, allowing users to perform custom cleanups.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we call xCloseCallback() after we've disconnected all virtual tables, though?
** with a NULL pointer as the second parameter. | ||
** ^The third parameter to [libsql_close_hook()] is passed through as | ||
** the first parameter to callbacks. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's mention somewhere that this is called with sqlite3 object mutex held so people know they can't call the sqlite API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh but they can and most likely will. The main db mutex is recursive so it's fine to reenter it from the same fiber: https://github.com/libsql/libsql/blob/c2cf4380bbf254ec7bd2f10c025a260a123bdcb5/src/main.c#L3296
Hm, no idea, I wanted to call it as early as possible to avoid the preexisting shutdown order issue. I can imagine some artificial use cases where you want to finalize some statements with virtual tables still existing. Again, no idea. I'm open to changing the order if presented an argument why it makes sense, otherwise I suggest leaving it as is until somebody complains? |
libsql_close_hook was added here in tursodatabase#219 but it is not available to loadable extensions. Following the same pattern was was done for `wal_methods_unregister` (tursodatabase@47e59b7) to expose this to loadable extensions. - Will be used in tursodatabase#434 to do cleanup of cr-sqlite
libsql_close_hook was added here in tursodatabase#219 but it is not available to loadable extensions. Following the same pattern was was done for `wal_methods_unregister` (tursodatabase@47e59b7) to expose this to loadable extensions. - Will be used in tursodatabase#434 to do cleanup of cr-sqlite
217: move pyhton sdk to packages r=MarinPostma a=MarinPostma housekeeping 218: add rust sdk cargo lock to gitignore r=MarinPostma a=MarinPostma housekeeping 219: remove mergify config r=MarinPostma a=MarinPostma housekeeping we don't use mergify anymore 220: remove jepsen boilerplate r=MarinPostma a=MarinPostma housekeeping. This is just boilerplate, and no actual tests exist. Let's bring them back if we think we need then later on. Co-authored-by: ad hoc <postma.marin@protonmail.com>
The callback allows registering a custom close hook, which gets called prior to closing the connection, allowing users to perform custom cleanups.
Fixes #62