http://sqlite.org/c3ref/busy_timeout.html

There can only be a single busy handler for a particular database connection at any given moment. If another busy handler was defined (using sqlite3_busy_handler()) prior to calling this routine, that other busy handler is cleared.

And also:
https://sqlite.org/pragma.html#pragma_busy_timeout

Each database connection can only have a single busy handler. This PRAGMA sets the busy handler for the process, possibly overwriting any previously set busy handler.

I have added a commit to eagerly free old busy handlers if busy_timeout is called. I have not addressed the PRAGMA case, but I assume that it is pretty rare for a user to both use PRAGMA and busy_timeout, and even in such cases, it seems unlikely that simply freeing a function later than strictly necessary would be a problem. I believe this would be hard to detect anyway since rusqlite does not parse its SQL…?

Idem:
http://sqlite.org/c3ref/wal_hook.html

A single database handle may have at most a single write-ahead log callback registered at one time. ... . Note that the sqlite3_wal_autocheckpoint() interface and the wal_autocheckpoint pragma both invoke sqlite3_wal_hook() and will overwrite any prior sqlite3_wal_hook() settings.

Maybe we should not use Fn* when we cannot know when to free them...

Idem:
http://sqlite.org/c3ref/trace_v2.html

Each call to either sqlite3_trace(D,X,P) or sqlite3_trace_v2(D,M,X,P) overrides (cancels) all prior calls to sqlite3_trace(D,X,P) or sqlite3_trace_v2(D,M,X,P) for the database connection D. Each database connection may have at most one trace callback.

https://sqlite.org/c3ref/profile.html

Invoking either sqlite3_trace() or sqlite3_trace_v2() will cancel the profile callback.

Maybe we should not use Fn* when we cannot know when to free them...

The only risk is that at most one function becomes freed at a slightly later point than is optimal. But I don’t think this risk is significant, since:

1. There is no possibility for unbounded growth, since there is at most one function is allocated at a time.
2. All the functions will be freed when the database conection is closed, so there is no global memory leak.
3. All existing usages of these functions are with function pointers, which generally come from non-capturing closures, and therefore won’t have a heap allocation that needs to be freed anyway (since ThinBoxAny doesn’t allocate for ZSTs).

I could update trace and profile to free the trace_v2 function, but at the same time it seems to me reasonable not to, since those functions are deprecated anyway. The probability that a user calls trace_v2 followed by trace and absolutely must free the closure allocated by trace_v2 seems very very small, to me.

2. All the functions will be freed when the database conection is closed, so there is no global memory leak.

Except if a connection pool is used which is more likely.

3. All existing usages of these functions are with function pointers, which generally come from non-capturing closures,

I don't understand: if there is non-capturing closures, currentfn should be fine, no ?

Except if a connection pool is used which is more likely.

Even if a connection pool is used, when that pool is pruned, or the pool itself is dropped, everything will be correctly freed. I mean, I would imagine that the overhead of keeping a DB connection alive is significantly greater than the overhead of one function that has potentially not been freed.

I don't understand: if there is non-capturing closures, current fn should be fine, no ?

Yes, if there are no captures then fn suffices. My point is that there is no additional resource overhead for existing users of these functions – the resource overhead strictly applies to new users. And even then, it seems unlikely to have a real-world impact.

In the vast majority of cases, users will set one callback and not override it, and so the question of whether this callback is freed is not important. In a small number of cases, users will change the callback through Rusqlite only, and thus it will be freed correctly. In another small number of cases, users will change the callback through first Rusqlite and then Sqlite itself, and in some of these cases it might be important that memory is freed eagerly. However, they can always do something like wrap closure’s state in a Weak to achieve this. As a result, I don’t think this is a performance footgun.

As a user I'd much rather take the risk of having the allocated Fn for longer if it means not having to do the ffi myself in order to have data in the callbacks

There is a side-effect of using FnMut instead of fn: you cannot register them on shared connection (loadable extension) because the ThinBoxAny / Option<Box<dyn FnMut don't live long enough.