diff --git a/faiss/impl/index_read.cpp b/faiss/impl/index_read.cpp index 69197a812..dce0805d0 100644 --- a/faiss/impl/index_read.cpp +++ b/faiss/impl/index_read.cpp @@ -2061,6 +2061,7 @@ std::unique_ptr read_index_up(IOReader* f, int io_flags) { auto idxp = std::make_unique(); read_index_header(*idxp, f); idxp->q1.quantizer = read_index(f, io_flags); + idxp->q1.own_fields = true; READ1(idxp->q1.nlist); READ1(idxp->q1.quantizer_trains_alone); read_ProductQuantizer(&idxp->pq, f); diff --git a/tests/test_read_index_deserialize.cpp b/tests/test_read_index_deserialize.cpp index 659420a90..b889e24cb 100644 --- a/tests/test_read_index_deserialize.cpp +++ b/tests/test_read_index_deserialize.cpp @@ -10,9 +10,11 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -1485,6 +1487,88 @@ TEST(ReadIndexDeserialize, HNSW2LevelWrongStorageType) { expect_read_throws_with(buf, "Index2Layer or IndexIVFPQ"); } +// ----------------------------------------------------------------------- +// Test: Index2Layer deserialization sets own_fields=true so the +// sub-quantizer is freed when the deserialized index is destroyed. +// ----------------------------------------------------------------------- + +struct CountedDestructorIndex : IndexFlatL2 { + static int destructor_count; + using IndexFlatL2::IndexFlatL2; + ~CountedDestructorIndex() override { + destructor_count++; + } +}; +int CountedDestructorIndex::destructor_count = 0; + +TEST(ReadIndexDeserialize, Index2LayerQuantizerOwnership) { + int d = 4, nb = 512, nlist = 2; + + // Build and serialize a valid Index2Layer. + auto original_q = std::make_unique(d); + Index2Layer original(original_q.release(), nlist, 2); + original.q1.own_fields = true; + std::vector xb(d * nb); + { + std::mt19937 rng(42); + std::uniform_real_distribution dist; + for (auto& v : xb) { + v = dist(rng); + } + } + original.train(nb, xb.data()); + original.add(nb, xb.data()); + + VectorIOWriter writer; + write_index(&original, &writer); + + // Deserialize and swap in a CountedDestructorIndex as the quantizer. + // When the deserialized index is destroyed, own_fields controls + // whether the quantizer is freed. The destructor counter verifies + // this independently of any flag check. + CountedDestructorIndex::destructor_count = 0; + { + VectorIOReader reader; + reader.data = writer.data; + auto idx = std::unique_ptr(read_index(&reader)); + auto* idx2l = dynamic_cast(idx.get()); + ASSERT_NE(idx2l, nullptr); + ASSERT_NE(idx2l->q1.quantizer, nullptr); + + // Replace the deserialized quantizer with a CountedDestructorIndex. + delete idx2l->q1.quantizer; + idx2l->q1.quantizer = new CountedDestructorIndex(d); + EXPECT_EQ(CountedDestructorIndex::destructor_count, 0); + // idx goes out of scope here. If own_fields is true, + // ~Level1Quantizer deletes the CountedDestructorIndex. + } + EXPECT_EQ(CountedDestructorIndex::destructor_count, 1); +} + +// ----------------------------------------------------------------------- +// Test: Index2Layer with a null quantizer (serialized as "null" fourcc) +// still sets own_fields=true. Deleting a nullptr is safe, so this +// verifies the fix doesn't assume the quantizer is non-null. +// ----------------------------------------------------------------------- +TEST(ReadIndexDeserialize, Index2LayerNullQuantizerOwnership) { + std::vector buf; + push_fourcc(buf, "Ix2L"); + push_index_header(buf, 4, 0); + // Null sub-quantizer. + push_fourcc(buf, "null"); + push_val(buf, size_t(1)); // nlist + // Truncate here — the next READ1 will fail and trigger cleanup. + // With own_fields=true and quantizer=nullptr, the destructor + // must not crash (delete nullptr is well-defined). + EXPECT_THROW( + { + VectorIOReader reader; + reader.data = buf; + read_index(&reader); + }, + FaissException); +} + // ----------------------------------------------------------------------- // Test: IndexHNSW2Level with null storage is rejected. // -----------------------------------------------------------------------