ソースを参照

Fix memory leaks. All tests pass.

Stefano Cossu 3 年 前
コミット
313203b673
3 ファイル変更46 行追加54 行削除
  1. 1 6
      src/environment.c
  2. 22 35
      src/store_htable.c
  3. 23 13
      src/store_mdb.c

+ 1 - 6
src/environment.c

@@ -35,12 +35,7 @@ static int term_cache_cmp_fn (const void *a, const void *b, void *udata)
 
 
 static void term_cache_free_fn (void *item)
-{
-    LSUP_KeyedTerm *kterm = item;
-    log_trace ("Freeing term %s", kterm->term->data);
-
-    LSUP_term_free (kterm->term);
-}
+{ LSUP_term_free (((LSUP_KeyedTerm *) item)->term); }
 
 
 /*

+ 22 - 35
src/store_htable.c

@@ -97,11 +97,7 @@ uint64_t trp_key_hash_fn (
 
 
 int trp_key_cmp_fn (const void *a, const void *b, void *udata)
-{
-    const LSUP_Key *aa = a, *bb = b;
-    log_trace ("cmp a: {%lx, %lx, %lx}", aa[0], aa[1], aa[2]);
-    log_trace ("cmp b: {%lx, %lx, %lx}", bb[0], bb[1], bb[2]);
-    return memcmp (a, b, TRP_KLEN); }
+{ return memcmp (a, b, TRP_KLEN); }
 
 
 /**
@@ -276,17 +272,16 @@ LSUP_htstore_size (const LSUP_HTStore *ht)
 LSUP_rc
 LSUP_htstore_add_term (HTStore *store, const LSUP_Buffer *sterm)
 {
-    if (hashmap_get (store->idx, sterm)) return LSUP_NOACTION;
+    IndexEntry entry_s = {
+        .key = LSUP_buffer_hash (sterm),
+    };
+    if (hashmap_get (store->idx, &entry_s)) return LSUP_NOACTION;
 
-    LSUP_Key tk = LSUP_buffer_hash (sterm);
-    log_trace ("Adding term key: %lx", tk);
+    entry_s.sterm = LSUP_buffer_new (sterm->size, sterm->addr);
 
-    hashmap_set (
-            store->idx, &(IndexEntry){
-                .key = tk,
-                // This shall be freed with the index hashmap.
-                .sterm = LSUP_buffer_new (sterm->size, sterm->addr)
-            });
+    log_trace ("Adding term key: %lx", entry_s.key);
+    hashmap_set (store->idx, &entry_s);
+    //log_trace ("Term index size: %lu", hashmap_count (store->idx));
 
     return LSUP_OK;
 }
@@ -317,10 +312,8 @@ LSUP_htstore_add_iter (HTIterator *it, const LSUP_BufferTriple *sspo)
 
     if (rc != LSUP_OK) return rc;
 
-    for (int i = 0; i < 3; i++) {
-        rc = LSUP_htstore_add_term (it->store, LSUP_btriple_pos (sspo, i));
-        if (rc != LSUP_OK) return rc;
-    }
+    for (int i = 0; i < 3; i++)
+        LSUP_htstore_add_term (it->store, LSUP_btriple_pos (sspo, i));
 
     return rc;
 }
@@ -336,37 +329,31 @@ LSUP_htstore_remove(
         LSUP_HTStore *store, const LSUP_Buffer *ss, const LSUP_Buffer *sp,
         const LSUP_Buffer *so,  size_t *ct_p)
 {
-    LSUP_rc rc;
-    size_t i, ct;
+    size_t ct;
 
     LSUP_HTIterator *it = LSUP_htstore_lookup (store, ss, sp, so, &ct);
     if (UNLIKELY (!it)) return LSUP_DB_ERR;
 
+    LSUP_rc rc;
     if (ct == 0) {
         rc = LSUP_NOACTION;
         goto finally;
     }
-    // Preallocate delete list.
-    LSUP_TripleKey **del_list = malloc (sizeof (*del_list) * ct);
-    if (UNLIKELY (!del_list)) {
-        rc = LSUP_MEM_ERR;
-        goto finally;
-    }
-
-    // Gather delete list first. Looping over hashmap while deleting elements
-    // won't work.
-    for (i = 0; htiter_next_key (it) == LSUP_OK; i++)
-        del_list[i] = it->entry;
 
-    for (i = 0; i < ct; i++)
-        hashmap_delete (store->keys, del_list[i]);
+    while (htiter_next_key (it) == LSUP_OK) {
+        log_trace (
+                "Deleting {%lx, %lx, %lx}.",
+                it->entry[0][0], it->entry[0][1], it->entry[0][2]);
+        hashmap_delete (store->keys, it->entry);
+        rc = LSUP_OK;
+        it->cur = 0; // Reset cursor, buckets are rearranged after deletion.
+    }
 
 finally:
     LSUP_htiter_free (it);
-
     if (ct_p) *ct_p = ct;
 
-    return LSUP_OK;
+    return rc;
 }
 
 

+ 23 - 13
src/store_mdb.c

@@ -728,8 +728,10 @@ mdbiter_next_key (LSUP_MDBIterator *it)
     db_rc = mdb_cursor_count (it->ctx_cur, &ct);
     if (db_rc != MDB_SUCCESS) return LSUP_DB_ERR;
     // 1 spare for sentinel. Always allocated even on zero matches.
-    it->ck = malloc (sizeof (*it->ck) * (ct + 1));
-    //log_trace("Slots allocated @%p: %lu of size %lu", it->ck, ct + 1, sizeof (*it->ck) * (ct + 1));
+    LSUP_Key *tmp_ck = realloc (it->ck, sizeof (*it->ck) * (ct + 1));
+    if (!tmp_ck) return LSUP_MEM_ERR;
+    it->ck = tmp_ck;
+
     size_t i = 0;
     do {
         //log_trace("Copying to slot #%lu @%p", i, it->ck + i);
@@ -790,13 +792,14 @@ LSUP_mdbiter_count (LSUP_MDBIterator *it)
 void
 LSUP_mdbiter_free (MDBIterator *it)
 {
-    if (it) {
-        if (it->cur) mdb_cursor_close (it->cur);
-        if (it->ctx_cur) mdb_cursor_close (it->ctx_cur);
-        if (it->store->txn != it->txn) mdb_txn_abort (it->txn);
+    if (!it) return;
 
-        free (it);
-    }
+    if (it->cur) mdb_cursor_close (it->cur);
+    if (it->ctx_cur) mdb_cursor_close (it->ctx_cur);
+    if (it->store->txn != it->txn) mdb_txn_abort (it->txn);
+    free (it->ck);
+
+    free (it);
 }
 
 
@@ -1443,6 +1446,7 @@ lookup_1bound (uint8_t idx0, MDBIterator *it, size_t *ct)
             ct_it->ctx_cur = it->ctx_cur;
             ct_it->key = it->key;
             ct_it->data = it->data;
+            ct_it->ck = NULL;
             ct_it->luk[0] = it->luk[0];
             ct_it->luc = it->luc;
             ct_it->i = 0;
@@ -1450,13 +1454,15 @@ lookup_1bound (uint8_t idx0, MDBIterator *it, size_t *ct)
             LSUP_rc rc = lookup_1bound (idx0, ct_it, NULL);
             if (rc < 0) return rc;
 
-            while (mdbiter_next_key (ct_it) != LSUP_END) {
+            LSUP_rc db_rc;
+            while (LSUP_END != (db_rc = mdbiter_next_key (ct_it))) {
+                if (UNLIKELY (db_rc < 0)) return db_rc;
                 (*ct)++;
-                log_trace ("Counter increased to %lu.", *ct);
             }
 
             // Free the counter iterator without freeing the shared txn.
             if (ct_it->cur) mdb_cursor_close (ct_it->cur);
+            free (ct_it->ck);
             free (ct_it);
 
         } else {
@@ -1540,18 +1546,22 @@ lookup_2bound(uint8_t idx0, uint8_t idx1, MDBIterator *it, size_t *ct)
             MDBIterator *ct_it;
             MALLOC_GUARD (ct_it, LSUP_MEM_ERR);
 
-            ct_it->luk[0] = it->luk[0];
-            ct_it->luk[1] = it->luk[1];
-            ct_it->luc = it->luc;
             ct_it->store = it->store;
             ct_it->txn = it->txn;
             ct_it->ctx_cur = it->ctx_cur;
+            ct_it->ck = NULL;
+            ct_it->luk[0] = it->luk[0];
+            ct_it->luk[1] = it->luk[1];
+            ct_it->luc = it->luc;
+            ct_it->i = 0;
+
             lookup_2bound (idx0, idx1, ct_it, NULL);
 
             while (mdbiter_next_key (ct_it) != LSUP_END) (*ct) ++;
 
             // Free the counter iterator without freeing the shared txn.
             if (ct_it->cur) mdb_cursor_close (ct_it->cur);
+            free (ct_it->ck);
             free (ct_it);
 
         } else {