After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 579364 - [sqlite3] Invalid free function
[sqlite3] Invalid free function
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: Bindings
0.7.x
Other All
: Normal normal
: ---
Assigned To: Vala maintainers
Vala maintainers
: 582962 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2009-04-18 01:11 UTC by sanpi
Modified: 2010-04-10 18:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Possible fix (2.11 KB, patch)
2009-07-04 21:01 UTC, Evan Nemerson
reviewed Details | Review

Description sanpi 2009-04-18 01:11:14 UTC
Please describe the problem:
Hello,

For this library, the functions for the memory management are sqlite3_malloc/sqlite3_realloc and sqlite3_free.

I have a segfault when my program try to free the memory with the g_free function (for example with the Sqlite.Database.exec when an error occur).

Currently, I have add the weak attribute for this parameter, but I have a memory leak.

Steps to reproduce:


Actual results:


Expected results:


Does this happen every time?


Other information:
Comment 1 Evan Nemerson 2009-05-02 06:44:34 UTC
I've run into similar problems in other libraries (tokyo cabinet comes to mind) where they return a common type (most often a string) that needs to be free()d with something other than g_free().

For sqlite, I see two ways to work around this. The first is to bind sqlite3_config(), and struct sqlite3_mem_methods (see http://www.sqlite.org/c3ref/mem_methods.html ), and use them to make sqlite call g_malloc/g_free.

The other way is to create a [CCode (free_function = "sqlite3_free")] sqlite_string : string { }, and just have everything that would return a string return an sqlite_string. Not pretty, but the memory method stuff is marked as experimental in sqlite, so this way might be more stable.

That said, since the problem isn't unique to sqlite, I wonder how this could/should be solved in other libraries. Would it be possible to have an attribute to override the free_function of the returned value? For example, something like

[CCode (return_free_function = "free")] string to_string ();

That instance of a string could then be freed with free() instead of g_free().
Comment 2 Jens Georg 2009-05-17 18:02:20 UTC
*** Bug 582962 has been marked as a duplicate of this bug. ***
Comment 3 Evan Nemerson 2009-07-04 20:57:09 UTC
I just did some more research on this, and I can't see a way to make the sqlite3_config solution I brought up work reliably. SQLite requires an xSize callback which will return the size of a buffer. Since there isn't a standard way to do this (malloc_usable_size on glibc and freebsd, _msize on windows, I don't think uclibc has anything), sqlite prepends an int64 length to each buffer it allocates (see http://www.sqlite.org/cvstrac/fileview?f=sqlite/src/mem1.c&v=1.30 ), so sqlite3_realloc and sqlite3_free really call realloc and free on a buffer 8 bytes before what you pass them.

The only case I see the sqlite3_config solution working is when g_mem_is_system_malloc() == true, and the first four sqlite3_mem_methods are set to (g_)malloc, (g_)realloc, (g_)free, and malloc_usable_size/_msize on a platform that supports it.
Comment 4 Evan Nemerson 2009-07-04 21:01:21 UTC
Created attachment 137855 [details] [review]
Possible fix

Adds a sqlite_string class which extends string, but is freed by sqlite3_free. As far as I can see, Sqlite.Database.exec and Sqlite.Database.get_table are the only functions that actually need to use it. I've also made Sqlite.Database.prepare(_v2)'s tail parameter weak, which I believe is the correct behaviour (sqlite docs could be clearer).
Comment 5 Jürg Billeter 2010-03-22 09:01:49 UTC
As it is appears to be used only for error messages in two functions, couldn't we just use an inline VAPI method that uses g_strdup (and char* with sqlite3_free for the sqlite version)?
Comment 6 Evan Nemerson 2010-04-10 18:45:11 UTC
commit 2bc4b792bc22fe387d00637ecdd1c78a8950d692
Author: Evan Nemerson <evan@coeus-group.com>
Date:   Sat Apr 10 11:41:08 2010 -0700

    sqlite3: add wrapper methods for sqlite3_exec and sqlite3_get_table
    
    Fixes bug 579364.