Skip to content

Conversation

@L1l1thLY
Copy link

@L1l1thLY L1l1thLY commented Feb 3, 2026

I found a few memory leaks in the code and attempted to fix them. Could you please take a look and confirm whether the fixes are correct? I’m not entirely sure, but the changes are as follows:

  1. Line 2537-2538 (vec_eachFilter): pzErrMsg allocated but not freed on error
  2. Line 1651-1652 (vec_slice): Early return without calling cleanup(vector)
  3. Line 1674-1675 (vec_slice): Early return without calling cleanup(vector)
  4. Line 5145-5152 (vec0_init): zSql not freed before goto error
  5. Line 7064 (vec0Filter_knn): knn_data not freed on cleanup when error occurs
  6. Line 8276-8278 (vec0Update_Insert): zSql not freed before goto cleanup
  7. Lines 9409-9468 (vec_static_blob_entriesFilter): Multiple memory leaks - err, knn_data, queryVector, topk_rowids, distances, candidates, taken not properly freed on errors

@L1l1thLY
Copy link
Author

L1l1thLY commented Feb 3, 2026

  1. vec0_free function (lines 3601-3638)

Issue: The function did not free partition column names, auxiliary column names, metadata column names, and metadata chunk shadow table names.

Fix: Added three loops to free this memory:

  • paritition_columns[i].name - partition column names
  • auxiliary_columns[i].name - auxiliary column names
  • shadowMetadataChunksNames[i] - metadata chunk shadow table names
  • metadata_columns[i].name - metadata column names
  1. vec0_init error handling (lines 5180-5182)

Issue: In the error path, only vec0_free(pNew) was called to free member variables, but the pNew structure itself was not freed.

Fix: Added sqlite3_free(pNew) after vec0_free(pNew).

  1. vec0Update_UpdateAuxColumn function (line 8666)

Issue: zSql allocated via sqlite3_mprintf was never freed.

Fix: Added sqlite3_free((void *)zSql) immediately after the sqlite3_prepare_v2 call.

  1. vec0_write_metadata_value function (lines 8108 and 8130)

Issue: zSql was not freed in two places:

  • SQL statement for UPDATE or INSERT operations
  • SQL statement for DELETE operations

Fix: Added sqlite3_free((void *)zSql) after each sqlite3_prepare_v2 call.

  1. vec0Update_Delete_ClearMetadata function (lines 8572 and 8580)

Issue:

  • zSql was not freed when sqlite3_prepare_v2 failed
  • stmt was not finalized when sqlite3_step failed

Fix:

  • Added sqlite3_free((void *)zSql) after the sqlite3_prepare_v2 call
  • Added sqlite3_finalize(stmt) in the error path

mceachen added a commit to photostructure/sqlite-vec that referenced this pull request Feb 10, 2026
- vec_eachFilter: free pzErrMsg on error to prevent leak
- vec_slice: use goto done instead of return in INT8 and BIT cases
  to ensure vector cleanup is called on malloc failure

These were the last 3 unfixed memory leaks from upstream PR asg017#258.
The other 4 issues from that PR were already addressed in previous commits.

Upstream-PR: asg017#258
mceachen added a commit to photostructure/sqlite-vec that referenced this pull request Feb 10, 2026
Add test-error-paths.py with 30 tests specifically targeting error-handling
code paths that previously had memory leaks (fixed in PR asg017#258).

These tests ensure that error paths are exercised by the test suite so that
memory leaks would be caught by sanitizers (ASan/LSan) if reintroduced.

Tests cover:
- vec_each with invalid inputs (NULL, wrong types, malformed JSON, empty arrays)
- vec_slice error conditions (NULL, invalid types, bad indices)
- vector_from_value error paths (mismatched types/dimensions in distance/add/sub)
- vec0 INSERT/KNN errors (NULL, dimension mismatches, type mismatches)
- Metadata operations with invalid inputs
- Repeated error operations to stress-test cleanup paths

Note: malloc failure paths (OOM conditions) are not tested as they require
fault injection (SQLITE_TESTCTRL_FAULT_INSTALL) which is not available in
the standard build. The fixes ensure proper cleanup via 'goto done' instead
of early 'return' statements.

Related: Upstream PR asg017#258
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant