Fix multiple memory leaks in vec0 operations (INSERT, DELETE, KNN queries)

stevemurr · claude · Oldes · commit 875f29af469a · 2026-02-23T09:55:50.000+01:00
This commit applies fixes from PR asg017#258 and additional memory leak fixes identified in issues asg017#245, asg017#259, and asg017#265. Memory leak fixes include: 1. vec_slice: Early return without calling cleanup(vector) - now uses goto done 2. vec_eachFilter: pzErrMsg allocated but not freed on error 3. vec0_free: Missing cleanup for partition, auxiliary, and metadata column names 4. vec0_init: zSql not freed before goto error, pNew not freed on error path 5. vec0_metadata_filter_text: rowids not freed on error after allocation 6. vec0Filter_knn: knn_data not freed on cleanup when error occurs, array cleanup missing on error paths for metadata in-list processing 7. vec0_write_metadata_value: zSql not freed after sqlite3_prepare_v2 8. vec0Update_Insert: zSql not freed after sqlite3_prepare_v2 9. vec0Update_Delete_ClearMetadata: zSql not freed, stmt not finalized on error 10. vec0Update_UpdateAuxColumn: zSql not freed after sqlite3_prepare_v2 11. vec_static_blob_entriesFilter: Complete rewrite to properly handle all error paths - knn_data, queryVector, topk_rowids, distances, candidates, taken not properly freed on errors These fixes address memory corruption and database corruption issues that occurred during vec0 INSERT/DELETE operations and KNN queries. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> (cherry picked from commit 12259fa)
diff --git a/sqlite-vec.c b/sqlite-vec.c
@@ -1649,7 +1649,7 @@ static void vec_slice(sqlite3_context *context, int argc,
     i8 *out = sqlite3_malloc(outSize);
     if (!out) {
       sqlite3_result_error_nomem(context);
-      return;
+      goto done;
     }
     memset(out, 0, outSize);
     for (size_t i = 0; i < n; i++) {
@@ -1672,7 +1672,7 @@ static void vec_slice(sqlite3_context *context, int argc,
     u8 *out = sqlite3_malloc(outSize);
     if (!out) {
       sqlite3_result_error_nomem(context);
-      return;
+      goto done;
     }
     memset(out, 0, outSize);
     for (size_t i = 0; i < n / CHAR_BIT; i++) {
@@ -2535,6 +2535,7 @@ static int vec_eachFilter(sqlite3_vtab_cursor *pVtabCursor, int idxNum,
   int rc = vector_from_value(argv[0], &pCur->vector, &pCur->dimensions,
                              &pCur->vector_type, &pCur->cleanup, &pzErrMsg);
   if (rc != SQLITE_OK) {
+    sqlite3_free(pzErrMsg);
     return SQLITE_ERROR;
   }
   pCur->iRowid = 0;
@@ -3616,6 +3617,24 @@ void vec0_free(vec0_vtab *p) {
     sqlite3_free(p->vector_columns[i].name);
     p->vector_columns[i].name = NULL;
   }
+
+  for (int i = 0; i < p->numPartitionColumns; i++) {
+    sqlite3_free(p->paritition_columns[i].name);
+    p->paritition_columns[i].name = NULL;
+  }
+
+  for (int i = 0; i < p->numAuxiliaryColumns; i++) {
+    sqlite3_free(p->auxiliary_columns[i].name);
+    p->auxiliary_columns[i].name = NULL;
+  }
+
+  for (int i = 0; i < p->numMetadataColumns; i++) {
+    sqlite3_free(p->shadowMetadataChunksNames[i]);
+    p->shadowMetadataChunksNames[i] = NULL;
+
+    sqlite3_free(p->metadata_columns[i].name);
+    p->metadata_columns[i].name = NULL;
+  }
 }
 
 int vec0_num_defined_user_columns(vec0_vtab *p) {
@@ -5143,6 +5162,7 @@ static int vec0_init(sqlite3 *db, void *pAux, int argc, const char *const *argv,
         goto error;
       }
       rc = sqlite3_prepare_v2(db, zSql, -1, &stmt, NULL);
+      sqlite3_free(zSql);
       if ((rc != SQLITE_OK) || (sqlite3_step(stmt) != SQLITE_DONE)) {
         sqlite3_finalize(stmt);
         *pzErr = sqlite3_mprintf(
@@ -5160,6 +5180,7 @@ static int vec0_init(sqlite3 *db, void *pAux, int argc, const char *const *argv,
 
 error:
   vec0_free(pNew);
+  sqlite3_free(pNew);
   return SQLITE_ERROR;
 }
 
@@ -6074,6 +6095,7 @@ int vec0_metadata_filter_text(vec0_vtab * p, sqlite3_value * value, const void *
   rc = sqlite3_blob_read(rowidsBlob, rowids, sqlite3_blob_bytes(rowidsBlob), 0);
   if(rc != SQLITE_OK) {
     sqlite3_blob_close(rowidsBlob);
+    sqlite3_free(rowids);
     return rc;
   }
   sqlite3_blob_close(rowidsBlob);
@@ -7114,12 +7136,14 @@ int vec0Filter_knn(vec0_cursor *pCur, vec0_vtab *p, int idxNum,
           i64 v = sqlite3_value_int64(entry);
           rc = array_append(&item.array, &v);
           if (rc != SQLITE_OK) {
+            array_cleanup(&item.array);
             goto cleanup;
           }
         }
 
         if (rc != SQLITE_DONE) {
           vtab_set_error(&p->base, "Error fetching next value in `x in (...)` integer expression");
+          array_cleanup(&item.array);
           goto cleanup;
         }
 
@@ -7139,17 +7163,33 @@ int vec0Filter_knn(vec0_cursor *pCur, vec0_vtab *p, int idxNum,
           entry.zString = sqlite3_mprintf("%.*s", n, s);
           if(!entry.zString) {
             rc = SQLITE_NOMEM;
+            // Clean up already-added text entries
+            for(size_t j = 0; j < item.array.length; j++) {
+              sqlite3_free(((struct Vec0MetadataInTextEntry*)item.array.z)[j].zString);
+            }
+            array_cleanup(&item.array);
             goto cleanup;
           }
           entry.n = n;
           rc = array_append(&item.array, &entry);
           if (rc != SQLITE_OK) {
+            sqlite3_free(entry.zString);
+            // Clean up already-added text entries
+            for(size_t j = 0; j < item.array.length; j++) {
+              sqlite3_free(((struct Vec0MetadataInTextEntry*)item.array.z)[j].zString);
+            }
+            array_cleanup(&item.array);
             goto cleanup;
           }
         }
 
         if (rc != SQLITE_DONE) {
           vtab_set_error(&p->base, "Error fetching next value in `x in (...)` text expression");
+          // Clean up text entries
+          for(size_t j = 0; j < item.array.length; j++) {
+            sqlite3_free(((struct Vec0MetadataInTextEntry*)item.array.z)[j].zString);
+          }
+          array_cleanup(&item.array);
           goto cleanup;
         }
 
@@ -7163,6 +7203,13 @@ int vec0Filter_knn(vec0_cursor *pCur, vec0_vtab *p, int idxNum,
 
     rc = array_append(aMetadataIn, &item);
     if(rc != SQLITE_OK) {
+      // Clean up item.array since it wasn't added to aMetadataIn
+      if(p->metadata_columns[item.metadata_idx].kind == VEC0_METADATA_COLUMN_KIND_TEXT) {
+        for(size_t j = 0; j < item.array.length; j++) {
+          sqlite3_free(((struct Vec0MetadataInTextEntry*)item.array.z)[j].zString);
+        }
+      }
+      array_cleanup(&item.array);
       goto cleanup;
     }
   }
@@ -7217,6 +7264,12 @@ int vec0Filter_knn(vec0_cursor *pCur, vec0_vtab *p, int idxNum,
 
   sqlite3_free(aMetadataIn);
 
+  // Free knn_data if not assigned to cursor (error case)
+  if (rc != SQLITE_OK && knn_data) {
+    vec0_query_knn_data_clear(knn_data);
+    sqlite3_free(knn_data);
+  }
+
   return rc;
 }
 
@@ -8190,6 +8243,7 @@ int vec0_write_metadata_value(vec0_vtab *p, int metadata_column_idx, i64 rowid,
         }
         sqlite3_stmt * stmt;
         rc = sqlite3_prepare_v2(p->db, zSql, -1, &stmt, NULL);
+        sqlite3_free((void *)zSql);
         if(rc != SQLITE_OK) {
           goto done;
         }
@@ -8211,6 +8265,7 @@ int vec0_write_metadata_value(vec0_vtab *p, int metadata_column_idx, i64 rowid,
         }
         sqlite3_stmt * stmt;
         rc = sqlite3_prepare_v2(p->db, zSql, -1, &stmt, NULL);
+        sqlite3_free((void *)zSql);
         if(rc != SQLITE_OK) {
           goto done;
         }
@@ -8409,6 +8464,7 @@ int vec0Update_Insert(sqlite3_vtab *pVTab, int argc, sqlite3_value **argv,
       goto cleanup;
     }
     rc = sqlite3_prepare_v2(p->db, zSql, -1, &stmt, NULL);
+    sqlite3_free(zSql);
     if(rc != SQLITE_OK) {
       goto cleanup;
     }
@@ -8651,12 +8707,14 @@ int vec0Update_Delete_ClearMetadata(vec0_vtab *p, int metadata_idx, i64 rowid, i
         }
         sqlite3_stmt * stmt;
         rc = sqlite3_prepare_v2(p->db, zSql, -1, &stmt, NULL);
+        sqlite3_free((void *)zSql);
         if(rc != SQLITE_OK) {
           goto done;
         }
         sqlite3_bind_int64(stmt, 1, rowid);
         rc = sqlite3_step(stmt);
         if(rc != SQLITE_DONE) {
+          sqlite3_finalize(stmt);
           rc = SQLITE_ERROR;
           goto done;
         }
@@ -8743,6 +8801,7 @@ int vec0Update_UpdateAuxColumn(vec0_vtab *p, int auxiliary_column_idx, sqlite3_v
     return SQLITE_NOMEM;
   }
   rc = sqlite3_prepare_v2(p->db, zSql, -1, &stmt, NULL);
+  sqlite3_free((void *)zSql);
   if(rc != SQLITE_OK) {
     return rc;
   }
@@ -9529,52 +9588,67 @@ static int vec_static_blob_entriesFilter(sqlite3_vtab_cursor *pVtabCursor,
   if (idxNum == VEC_SBE__QUERYPLAN_KNN) {
     assert(argc == 2);
     pCur->query_plan = VEC_SBE__QUERYPLAN_KNN;
-    struct sbe_query_knn_data *knn_data;
+    struct sbe_query_knn_data *knn_data = NULL;
+    void *queryVector = NULL;
+    vector_cleanup cleanup = vector_cleanup_noop;
+    i32 *topk_rowids = NULL;
+    f32 *distances = NULL;
+    u8 *candidates = NULL;
+    u8 *taken = NULL;
+    int rc = SQLITE_OK;
+
     knn_data = sqlite3_malloc(sizeof(*knn_data));
     if (!knn_data) {
-      return SQLITE_NOMEM;
+      rc = SQLITE_NOMEM;
+      goto knn_cleanup;
     }
     memset(knn_data, 0, sizeof(*knn_data));
 
-    void *queryVector;
     size_t dimensions;
     enum VectorElementType elementType;
-    vector_cleanup cleanup;
     char *err;
-    int rc = vector_from_value(argv[0], &queryVector, &dimensions, &elementType,
+    rc = vector_from_value(argv[0], &queryVector, &dimensions, &elementType,
                                &cleanup, &err);
     if (rc != SQLITE_OK) {
-      return SQLITE_ERROR;
+      sqlite3_free(err);
+      rc = SQLITE_ERROR;
+      goto knn_cleanup;
     }
     if (elementType != p->blob->element_type) {
-      return SQLITE_ERROR;
+      rc = SQLITE_ERROR;
+      goto knn_cleanup;
     }
     if (dimensions != p->blob->dimensions) {
-      return SQLITE_ERROR;
+      rc = SQLITE_ERROR;
+      goto knn_cleanup;
     }
 
     i64 k = min(sqlite3_value_int64(argv[1]), (i64)p->blob->nvectors);
     if (k < 0) {
       // HANDLE https://github.com/asg017/sqlite-vec/issues/55
-      return SQLITE_ERROR;
+      rc = SQLITE_ERROR;
+      goto knn_cleanup;
     }
     if (k == 0) {
       knn_data->k = 0;
       pCur->knn_data = knn_data;
+      cleanup(queryVector);
       return SQLITE_OK;
     }
 
     size_t bsize = (p->blob->nvectors + 7) & ~7;
 
-    i32 *topk_rowids = sqlite3_malloc(k * sizeof(i32));
+    topk_rowids = sqlite3_malloc(k * sizeof(i32));
     if (!topk_rowids) {
       // HANDLE https://github.com/asg017/sqlite-vec/issues/55
-      return SQLITE_ERROR;
+      rc = SQLITE_ERROR;
+      goto knn_cleanup;
     }
-    f32 *distances = sqlite3_malloc(bsize * sizeof(f32));
+    distances = sqlite3_malloc(bsize * sizeof(f32));
     if (!distances) {
       // HANDLE https://github.com/asg017/sqlite-vec/issues/55
-      return SQLITE_ERROR;
+      rc = SQLITE_ERROR;
+      goto knn_cleanup;
     }
 
     for (size_t i = 0; i < p->blob->nvectors; i++) {
@@ -9583,11 +9657,17 @@ static int vec_static_blob_entriesFilter(sqlite3_vtab_cursor *pVtabCursor,
       distances[i] =
           distance_l2_sqr_float(v, (float *)queryVector, &p->blob->dimensions);
     }
-    u8 *candidates = bitmap_new(bsize);
-    assert(candidates);
+    candidates = bitmap_new(bsize);
+    if (!candidates) {
+      rc = SQLITE_NOMEM;
+      goto knn_cleanup;
+    }
 
-    u8 *taken = bitmap_new(bsize);
-    assert(taken);
+    taken = bitmap_new(bsize);
+    if (!taken) {
+      rc = SQLITE_NOMEM;
+      goto knn_cleanup;
+    }
 
     bitmap_fill(candidates, bsize);
     for (size_t i = bsize; i >= p->blob->nvectors; i--) {
@@ -9601,6 +9681,21 @@ static int vec_static_blob_entriesFilter(sqlite3_vtab_cursor *pVtabCursor,
     knn_data->rowids = topk_rowids;
 
     pCur->knn_data = knn_data;
+
+    // Cleanup temporary allocations (not owned by knn_data)
+    sqlite3_free(candidates);
+    sqlite3_free(taken);
+    cleanup(queryVector);
+    return SQLITE_OK;
+
+knn_cleanup:
+    sqlite3_free(knn_data);
+    sqlite3_free(topk_rowids);
+    sqlite3_free(distances);
+    sqlite3_free(candidates);
+    sqlite3_free(taken);
+    cleanup(queryVector);
+    return rc;
   } else {
     pCur->query_plan = VEC_SBE__QUERYPLAN_FULLSCAN;
     pCur->iRowid = 0;
diff --git a/test_memory_fix.sql b/test_memory_fix.sql
@@ -0,0 +1,51 @@
+-- Test case for memory leak fixes in sqlite-vec
+-- This tests the fixes from PR #258 addressing issues #245, #259, #265
+
+-- Test 1: Basic vec0 table creation and insertion
+CREATE VIRTUAL TABLE test_vectors USING vec0(
+    id TEXT PRIMARY KEY,
+    embedding FLOAT[4]
+);
+
+-- Insert several vectors (this used to cause segfault on 5th+ insertion in some cases)
+INSERT INTO test_vectors(id, embedding) VALUES ('vec1', '[1.0, 2.0, 3.0, 4.0]');
+INSERT INTO test_vectors(id, embedding) VALUES ('vec2', '[5.0, 6.0, 7.0, 8.0]');
+INSERT INTO test_vectors(id, embedding) VALUES ('vec3', '[9.0, 10.0, 11.0, 12.0]');
+INSERT INTO test_vectors(id, embedding) VALUES ('vec4', '[13.0, 14.0, 15.0, 16.0]');
+INSERT INTO test_vectors(id, embedding) VALUES ('vec5', '[17.0, 18.0, 19.0, 20.0]');
+INSERT INTO test_vectors(id, embedding) VALUES ('vec6', '[21.0, 22.0, 23.0, 24.0]');
+INSERT INTO test_vectors(id, embedding) VALUES ('vec7', '[25.0, 26.0, 27.0, 28.0]');
+INSERT INTO test_vectors(id, embedding) VALUES ('vec8', '[29.0, 30.0, 31.0, 32.0]');
+INSERT INTO test_vectors(id, embedding) VALUES ('vec9', '[33.0, 34.0, 35.0, 36.0]');
+INSERT INTO test_vectors(id, embedding) VALUES ('vec10', '[37.0, 38.0, 39.0, 40.0]');
+
+SELECT 'Inserted 10 vectors successfully' AS status;
+
+-- Test 2: KNN query (tests memory handling in vec0Filter_knn)
+SELECT id, distance
+FROM test_vectors
+WHERE embedding MATCH '[1.0, 2.0, 3.0, 4.0]'
+ORDER BY distance
+LIMIT 3;
+
+-- Test 3: Delete operation (tests memory handling in vec0Update_Delete)
+DELETE FROM test_vectors WHERE id = 'vec5';
+SELECT 'Deleted vec5 successfully' AS status;
+
+-- Test 4: Verify integrity after operations
+SELECT COUNT(*) AS remaining_count FROM test_vectors;
+
+-- Test 5: Multiple insertions and deletions (stress test for memory leaks)
+INSERT INTO test_vectors(id, embedding) VALUES ('stress1', '[1.0, 1.0, 1.0, 1.0]');
+INSERT INTO test_vectors(id, embedding) VALUES ('stress2', '[2.0, 2.0, 2.0, 2.0]');
+DELETE FROM test_vectors WHERE id = 'stress1';
+INSERT INTO test_vectors(id, embedding) VALUES ('stress3', '[3.0, 3.0, 3.0, 3.0]');
+DELETE FROM test_vectors WHERE id = 'stress2';
+INSERT INTO test_vectors(id, embedding) VALUES ('stress4', '[4.0, 4.0, 4.0, 4.0]');
+DELETE FROM test_vectors WHERE id = 'stress3';
+
+SELECT 'Stress test passed' AS status;
+
+-- Clean up
+DROP TABLE test_vectors;
+SELECT 'All tests completed successfully' AS status;
