diff --git a/HashTable.cpp b/HashTable.cpp index c7f878a..b1dfc92 100644 --- a/HashTable.cpp +++ b/HashTable.cpp @@ -59,6 +59,12 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl { while (keyUpdates > 0) { if (iter == map.end()) { iter = map.begin(); + // The map is empty, so there are no more entries to garbage-collect. + // Break rather than spinning forever (keyUpdates is charged at 2x + // the entry count, so it may still be > 0 after emptying the map). + if (iter == map.end()) { + break; + } } for (; iter != map.end(); --keyUpdates) { if (iter->second <= oldestVersion) { diff --git a/test_conflict_set.py b/test_conflict_set.py index c530b79..67a5780 100644 --- a/test_conflict_set.py +++ b/test_conflict_set.py @@ -68,6 +68,25 @@ def test_hash_table_getBytes(): assert cs.check(read(0, b"key")) == [Result.CONFLICT] +def test_hash_table_setOldestVersion_empties_map(): + # Regression test for issue #78: setOldestVersion must terminate when + # its garbage-collection pass erases every entry in the map. Each write + # charges keyUpdates += 2, so with 50 writes keyUpdates = 100 and GC is + # enabled; erasing all entries leaves keyUpdates > 0 while the map is + # empty, which previously caused the GC loop to spin forever. + with ConflictSet(0, build_dir=build_dir, implementation="hash_table") as cs: + for i in range(50): + cs.addWrites(1, write(("k%06d" % i).encode())) + # All entries have version 1 <= oldestVersion 1 -> all erased. + cs.setOldestVersion(1) + # If we reach this point, setOldestVersion returned instead of hanging. + # The keys were erased, so reads at a valid version commit. + assert cs.check(read(1, b"k000000")) == [Result.COMMIT] + assert cs.check(read(2, b"k000000")) == [Result.COMMIT] + # A read older than oldestVersion is TooOld. + assert cs.check(read(0, b"k000000")) == [Result.TOO_OLD] + + def test_write_read_without_outer_reference(): # Regression test for issue #42: WriteRange/ReadRange must keep their # backing key buffers alive, because the C library reads the pointer