create_module_v2 destructor called too early?
(1) By Roger Binns (rogerbinns) on 2023-01-24 23:34:47 [source]
I believe the destructor supplied to create_module_v2 is called too early. Specifically it is called before the xDisconnect method of the module is called, and should be called after.
This behaviour is from sqlite3VtabUnlock() which calls sqlite3VtabModuleUnref() that calls that destructor, and sqlite3VtabUnlock() then calls xDisconnect.
It is causing specific problems for me because I am dynamically allocating sqlite3_module and freeing it in the destructor, with the xDisconnect lookup then being a dereference in freed memory.
(2) By Gunter Hick (gunter_hick) on 2023-01-25 06:49:54 in reply to 1 [link] [source]
I have noticed this behaviour too. I worked around it by making sure the module structure was allocated once and never freed. Same goes for the sqlite3_vtab structure.
(3) By David Jones (vman59) on 2023-01-25 12:27:49 in reply to 1 [link] [source]
The documentation says the final argument in create_module_v2 is a destructor for the client data pointer. A different page (https://sqlite.org/c3ref/module.html) says the module argument is a pointer to a persistent instance of a sqlite3_module structure.
(4) By Roger Binns (rogerbinns) on 2023-01-25 14:54:10 in reply to 3 [link] [source]
I understand that SQLite is behaving substantially in accordance with the documentation for an interface that was originally designed and exercised only by C code using a lot of static allocation.
That doesn't mean it is the optimal or expected thing to do.
I believe the destructor should be called after SQLite's last use of the module, not one line before the last use. It is less surprising behaviour and allows for languages using dynamic allocation.
I was using static data structures. The reason I had to change to dynamic is to support eponymous modules, and eponymous only modules, and modules where writable functions (update etc) are NULL, and 3 different values for iVersion.
(5) By Richard Hipp (drh) on 2023-01-25 19:07:25 in reply to 1 [link] [source]
Fixed on trunk. The patch will be in the next release.