-
Notifications
You must be signed in to change notification settings - Fork 8
Notes on 4store Memory Leak
fixed as of 2009/01/14
In the process of developing and testing the Py4s bindings for 4store, I discovered a memory leak in the 4store client code. This does not normally show up with, for example the 4s-httpd server since the query data structures are allocated by a child process and the child’s memory is freed when it exits. It does likely show up with 4s-query if it were used in an interactive session to execute many queries. The memory leak is particularly nasty for Py4s since it means that executing multiple queries will sooner or later (sooner) exhaust all available RAM.
A simple example program that duplicates the problem: memory_test.c It takes one command line argument, the number of times to run the loop.
Running under valgrind at Steve Harris’ suggestion produces results that point at rid_vector and binding memory management.
valgrind --leak-check=full ./memory_test 1000
There are several apparent leaks (see full valgrind output here) but the most egregious are:
- defining -DDEBUG_RV_ALLOC -DDEBUG_MERGE at compile time elicits helpful information. The existence of the DEBUG_RV_ALLOC points to someone having gone down this path.
- there are definitely some rid_vectors that are not getting freed properly, they are the ones that are allocated at query-datatypes.c:180 Notice how the objects at 0xd09230, 0xd093f0, 0xd09510, 0xd09630 and 0xd09c50 are never freed:
- up the call stack, calls to fs_binding_copy_and_clear in fs_query_handle_triple() at query.c:1559 and presumably in fs_query_handle_triple_multi() at query.c:1682 are suspicious: the pointer oldb is never freed in this function.
- adding fs_binding_free(oldb) before the return statements in fs_query_handle_triple() causes most of the memory in question to be freed – leak gone! however unfortunately it also causes 4s-query to segfault in the unit tests.
- adding debugging hooks to fs_binding_new and fs_binding_free seem to suggest that it is never used to free the memory referred to by oldb. Definitely something suspicious here.
- modified memory_test.c to add a test with a query that returns values, as opposed to just an ASK query or a SELECT that returns no results. The major memory leak seems to disappear.
- It seems as though the routine responsible for freeing the mysterious oldb is actually process_results() in query.c and in the case that the query didn’t fail, but nevertheless returned no bindings, it doesn’t free oldb. This patch solves the problem: however it remains that probably it should not be this routine’s responsibility to free the oldb at all but should be the calling code… Time to track down the more minor memory leaks, but that was the one that was show-stoppingly bad…