API for extension destructors

[psarna]

Following an interesting thread: https://sqlite.org/forum/forumpost/abf074d0d5ef87e1, we should consider extending the API in order to allow extension/user-defined function developers to provide destructors for their modules. Such destructors would automatically get called once the database connection to which they're bound gets closed.

[tantaman]

Hey, curious if any progress has been made here. I need it for cr-sqlite to be able to fully clean itself up when enabled on Turso. Otherwise Turso ooms since each connection leaves around prepared statements :(

[psarna]

No progress so far, but I'll take another shot at it soon, thanks for bumping!

[psarna]

@tantaman I'm reading through https://www.sqlite.org/c3ref/create_function.html and I need clarification -- why isn't xDestroy from sqlite3_create_function_v2 not enough for that purpose? I also checked https://www.sqlite.org/c3ref/create_module.html and modules allow you to load a destructor as well. Do I understand the issue correctly that the dangling prepared statements are created per connection, so what we actually need is a handler that gets called each time a connection is closed?

If so, I have the following conclusions:

1. In case of https://github.com/libsql/sqld, we should be robust and use https://www.sqlite.org/c3ref/next_stmt.html to go over all dangling statements, force-finalizing them right before calling sqlite3_close. That guards us against all kinds of leaks in extensions we load. /cc @MarinPostma @penberg I'd appreciate your thoughts here
2. Maybe we want a generic sqlite3_close_hook instead, that allows users to register their own hook that gets called on close? Then the extensions can use the generic mechanism, and it won't be specific to creating functions or modules. It will also leave the existing APIs intact, so it sounds overall better for backward compatibility

[psarna]

We could also add sqlite3_close_v3 that auto-closes statements, it doesn't seem like a particularly costly operation

[tantaman]

It seems like an ordering problem on SQLite's side.

From the comments in the thread (edited for clarity):

sqlite3_close checks for unfinalized statements (and aborts the close) before calling destructors. You can't finalize statements in the xDestroy method because the method won't be called.

And this has been my experience as well.

I have a few long-lived prepared statements that I stick into the extension's AuxData that I'd like to finalize only when the extension is unloaded. Re-preparing the statements every single transaction would be pretty expensive.

---

A sqlite3_close_hook close hook sounds like a good option. That way the extension can finalize the statements then have its xDestroy method called in the normal way.

The only issue with sqlite3_close_v3 is that it'd make it harder for developers to track down if they're leaking prepared statements. They'd have to understand which prepared statements are supposed to be there at close time vs which statements were actually leaked.

[psarna]

@tantaman just pushed https://github.com/libsql/libsql/pull/219, let me know if it solves your shutdown order issue. The close hook is fired as the first thing in sqlite3Close(), and in particular it happens before a call to connectionIsBusy which checks for dangling prepared statements, so it should be enough to perform a cleanup.