trealla-prolog / trealla

A compact, efficient Prolog interpreter written in plain-old C.
MIT License
252 stars 11 forks source link

query_destroy doesn't clear cache? #516

Closed guregu closed 3 months ago

guregu commented 3 months ago

As of the latest commit, a leak:

$ make clean && make -j8 debug && valgrind --leak-check=full ./tpl -g halt
==5815== Memcheck, a memory error detector
==5815== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==5815== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==5815== Command: ./tpl -g halt
==5815== 
==5815== 
==5815== HEAP SUMMARY:
==5815==     in use at exit: 1,728,288 bytes in 18 blocks
==5815==   total heap usage: 44,752 allocs, 44,734 frees, 150,961,352 bytes allocated
==5815== 
==5815== 1,728,288 (288 direct, 1,728,000 indirect) bytes in 9 blocks are definitely lost in loss record 2 of 2
==5815==    at 0x4889FB4: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-arm64-linux.so)
==5815==    by 0x233B7B: alloc_on_cache (heap.c:141)
==5815==    by 0x2354DB: prepare_call (heap.c:445)
==5815==    by 0x1829CF: bif_iso_notunify_2 (bif_predicates.c:250)
==5815==    by 0x2866E7: start (query.c:1638)
==5815==    by 0x2871B7: execute (query.c:1850)
==5815==    by 0x24EF87: dcg_expansion (parser.c:1714)
==5815==    by 0x24F25F: term_expansion (parser.c:1765)
==5815==    by 0x257743: tokenize (parser.c:3325)
==5815==    by 0x2400AB: load_text (module.c:1774)
==5815==    by 0x27E06B: pl_create (prolog.c:782)
==5815==    by 0x1179DF: main (tpl.c:172)
==5815== 
==5815== LEAK SUMMARY:
==5815==    definitely lost: 288 bytes in 9 blocks
==5815==    indirectly lost: 1,728,000 bytes in 9 blocks
==5815==      possibly lost: 0 bytes in 0 blocks
==5815==    still reachable: 0 bytes in 0 blocks
==5815==         suppressed: 0 bytes in 0 blocks
==5815== 
==5815== For lists of detected and suppressed errors, rerun with: -s
==5815== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Quick fix, but not sure if correct:

diff --git a/src/query.c b/src/query.c
index a9e98cf4..50fe5007 100644
--- a/src/query.c
+++ b/src/query.c
@@ -1884,6 +1884,18 @@ void query_destroy(query *q)
                free(save);
        }

+       for (page *a = q->cache_pages; a;) {
+               cell *c = a->cells;
+
+               for (pl_idx i = 0; i < a->max_idx_used; i++, c++)
+                       unshare_cell(c);
+
+               page *save = a;
+               a = a->next;
+               free(save->cells);
+               free(save);
+       }
+
        for (int i = 0; i < MAX_QUEUES; i++) {
                cell *c = q->queue[i];

After fix it looks good:

$ make clean && make -j8 debug && valgrind --leak-check=full ./tpl -g halt
==6516== Memcheck, a memory error detector
==6516== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==6516== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==6516== Command: ./tpl -g halt
==6516== 
==6516== 
==6516== HEAP SUMMARY:
==6516==     in use at exit: 0 bytes in 0 blocks
==6516==   total heap usage: 44,752 allocs, 44,752 frees, 150,961,352 bytes allocated
==6516== 
==6516== All heap blocks were freed -- no leaks are possible
==6516== 
==6516== For lists of detected and suppressed errors, rerun with: -s
==6516== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
guregu commented 3 months ago

Thanks!