From a038b7039368e9e90a2f016241ee5b9cdd2250e9 Mon Sep 17 00:00:00 2001 From: Sammy Aknan Date: Fri, 13 Mar 2026 16:29:40 -0400 Subject: [PATCH 1/6] fix: address potential race conditions --- .../net/staticstudios/data/DataManager.java | 200 ++++++------------ .../data/util/DependencyTrackingCache.java | 97 +++++++++ 2 files changed, 159 insertions(+), 138 deletions(-) create mode 100644 core/src/main/java/net/staticstudios/data/util/DependencyTrackingCache.java diff --git a/core/src/main/java/net/staticstudios/data/DataManager.java b/core/src/main/java/net/staticstudios/data/DataManager.java index 5c3a743..48b514f 100644 --- a/core/src/main/java/net/staticstudios/data/DataManager.java +++ b/core/src/main/java/net/staticstudios/data/DataManager.java @@ -1,8 +1,5 @@ package net.staticstudios.data; -import com.github.benmanes.caffeine.cache.Cache; -import com.github.benmanes.caffeine.cache.Caffeine; -import com.github.benmanes.caffeine.cache.RemovalCause; import com.google.common.base.Preconditions; import com.google.common.collect.MapMaker; import net.staticstudios.data.impl.DataAccessor; @@ -38,8 +35,6 @@ import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicLong; import java.util.function.Consumer; @ApiStatus.Internal @@ -65,12 +60,8 @@ public class DataManager { private final Set registeredUpdateHandlersForRedis = ConcurrentHashMap.newKeySet(); private final Set registeredChangeHandlersForCollection = ConcurrentHashMap.newKeySet(); private final Set registeredUpdateHandlersForReference = ConcurrentHashMap.newKeySet(); - private final Cache relationCache; - private final Map> dependencyToRelationCacheMapping = new ConcurrentHashMap<>(); - private final AtomicLong relationCacheGeneration = new AtomicLong(); - private final Cache cellCache; - private final Map> dependencyToCellCacheMapping = new ConcurrentHashMap<>(); - private final AtomicLong cellCacheGeneration = new AtomicLong(); + private final DependencyTrackingCache relationCache; + private final DependencyTrackingCache cellCache; private final List> valueSerializers = new CopyOnWriteArrayList<>(); private final Consumer updateHandlerExecutor; @@ -108,18 +99,8 @@ public DataManager(StaticDataConfig config, boolean setGlobal) { sqlBuilder = new SQLBuilder(this); dataAccessor = new H2DataAccessor(this, postgresListener, redisListener, taskQueue); - this.relationCache = Caffeine.newBuilder() - .maximumSize(10_000) - .expireAfterWrite(5, TimeUnit.MINUTES) - .removalListener((SelectQuery selectQuery, ReadCacheResult result, RemovalCause cause) -> cleanupRelationCacheEntry(selectQuery, result)) - .executor(Runnable::run) - .build(); - this.cellCache = Caffeine.newBuilder() - .maximumSize(20_000) - .expireAfterWrite(5, TimeUnit.MINUTES) - .removalListener((SelectQuery selectQuery, ReadCacheResult result, RemovalCause cause) -> cleanupCellCacheEntry(selectQuery, result)) - .executor(Runnable::run) - .build(); + this.relationCache = new DependencyTrackingCache("relation", 10_000, 5); + this.cellCache = new DependencyTrackingCache("cell", 20_000, 5); //todo: when we reconnect to postgres, refresh the internal cache from the source } @@ -868,8 +849,12 @@ public void registerReferenceUpdateHandlers(ReferenceMetadata metadata, Collecti public List> getUpdateHandlers(String schema, String table, String column, Class holderClass) { String key = schema + "." + table + "." + column; - if (persistentValueUpdateHandlers.containsKey(key) && persistentValueUpdateHandlers.get(key).containsKey(holderClass.getName())) { - return persistentValueUpdateHandlers.get(key).get(holderClass.getName()); + Map>> handlersForColumn = persistentValueUpdateHandlers.get(key); + if (handlersForColumn != null) { + List> handlers = handlersForColumn.get(holderClass.getName()); + if (handlers != null) { + return handlers; + } } return Collections.emptyList(); } @@ -1017,15 +1002,21 @@ public void handleDelete(List columnNames, String schema, String table, Preconditions.checkArgument(found, "Not all ID columnsInReferringTable were provided for UniqueData class %s. Required: %s, Provided: %s", uniqueDataMetadata.clazz().getName(), uniqueDataMetadata.idColumns(), Arrays.toString(values)); } - UniqueData instance = uniqueDataInstanceCache.getOrDefault(uniqueDataMetadata.clazz().getName(), Collections.emptyMap()).get(new ColumnValuePairs(idColumns)); - if (instance == null) { + Map classCache = uniqueDataInstanceCache.get(uniqueDataMetadata.clazz().getName()); + if (classCache == null) { return; } - instance.markDeleted(); + synchronized (classCache) { + UniqueData instance = classCache.get(new ColumnValuePairs(idColumns)); + if (instance == null) { + return; + } + instance.markDeleted(); + } }); } - public synchronized void updateIdColumns(List columnNames, String schema, String table, String column, Object[] oldValues, Object[] newValues) { + public void updateIdColumns(List columnNames, String schema, String table, String column, Object[] oldValues, Object[] newValues) { uniqueDataMetadataMap.values().forEach(uniqueDataMetadata -> { if (!uniqueDataMetadata.schema().equals(schema) || !uniqueDataMetadata.table().equals(table)) { return; @@ -1058,7 +1049,7 @@ public synchronized void updateIdColumns(List columnNames, String schema Preconditions.checkArgument(found, "Not all ID columnsInReferringTable were provided for UniqueData class %s. Required: %s, Provided: %s", uniqueDataMetadata.clazz().getName(), uniqueDataMetadata.idColumns(), Arrays.toString(oldValues)); } if (Arrays.equals(oldIdColumns, newIdColumns)) { - return; // no change to id columnsInReferringTable here + return; } ColumnValuePairs oldIdCols = new ColumnValuePairs(oldIdColumns); @@ -1066,13 +1057,15 @@ public synchronized void updateIdColumns(List columnNames, String schema if (classCache == null) { return; } - UniqueData instance = classCache.remove(oldIdCols); - if (instance == null) { - return; + synchronized (classCache) { + UniqueData instance = classCache.remove(oldIdCols); + if (instance == null) { + return; + } + ColumnValuePairs newIdCols = new ColumnValuePairs(newIdColumns); + instance.setIdColumns(newIdCols); + classCache.put(newIdCols, instance); } - ColumnValuePairs newIdCols = new ColumnValuePairs(newIdColumns); - instance.setIdColumns(newIdCols); - classCache.put(newIdCols, instance); }); } @@ -1145,10 +1138,13 @@ public T getInstance(Class clazz, @NotNull ColumnValue T instance; Map classCache = uniqueDataInstanceCache.get(clazz.getName()); - if (classCache != null && (instance = (T) classCache.get(idColumns)) != null) { - logger.trace("Cache hit for UniqueData class {} with ID columnsInReferringTable {}", clazz.getName(), idColumns); - if (!instance.isDeleted()) { - return instance; + if (classCache != null) { + synchronized (classCache) { + instance = (T) classCache.get(idColumns); + if (instance != null && !instance.isDeleted()) { + logger.trace("Cache hit for UniqueData class {} with ID columnsInReferringTable {}", clazz.getName(), idColumns); + return instance; + } } } @@ -1215,8 +1211,14 @@ public T getInstance(Class clazz, @NotNull ColumnValue PersistentManyToManyCollectionImpl.delegate(instance); PersistentOneToManyValueCollectionImpl.delegate(instance); - uniqueDataInstanceCache.computeIfAbsent(clazz.getName(), k -> new MapMaker().weakValues().makeMap()) - .put(idColumns, instance); + Map cache = uniqueDataInstanceCache.computeIfAbsent(clazz.getName(), k -> new MapMaker().weakValues().makeMap()); + synchronized (cache) { + T existing = (T) cache.get(idColumns); + if (existing != null && !existing.isDeleted()) { + return existing; + } + cache.put(idColumns, instance); + } logger.trace("Cache miss for UniqueData class {} with ID columnsInReferringTable {}. Created new instance.", clazz.getName(), idColumns); @@ -1726,107 +1728,47 @@ public void flushTaskQueue() { public StaticDataStatistics getStatistics() { StaticDataStatistics stats = new StaticDataStatistics(); dataAccessor.populateStatistics(stats); - stats.setRelationCacheSize((int) relationCache.estimatedSize()); - stats.setDependenciesToRelationsCacheMappingSize(dependencyToRelationCacheMapping.size()); - stats.setCellCacheSize((int) cellCache.estimatedSize()); - stats.setDependenciesToCellCacheMappingSize(dependencyToCellCacheMapping.size()); + stats.setRelationCacheSize(relationCache.estimatedSize()); + stats.setDependenciesToRelationsCacheMappingSize(relationCache.dependencyMappingSize()); + stats.setCellCacheSize(cellCache.estimatedSize()); + stats.setDependenciesToCellCacheMappingSize(cellCache.dependencyMappingSize()); return stats; } public @Nullable ReadCacheResult getRelationCacheResult(SelectQuery query) { - return relationCache.getIfPresent(query); + return relationCache.get(query); } public long getRelationCacheGeneration() { - return relationCacheGeneration.get(); + return relationCache.getGeneration(); } public void putRelationCacheResult(SelectQuery query, @NotNull ReadCacheResult result, long expectedGeneration) { - if (relationCacheGeneration.get() != expectedGeneration) { - return; - } - logger.trace("Putting result in relation cache for query {} with result {}", query, result); - relationCache.put(query, result); - for (Cell cell : result.getDependencies()) { - dependencyToRelationCacheMapping.computeIfAbsent(cell, k -> ConcurrentHashMap.newKeySet()) - .add(query); - } - if (relationCacheGeneration.get() != expectedGeneration) { - relationCache.invalidate(query); - cleanupRelationCacheEntry(query, result); - } + relationCache.put(query, result, expectedGeneration); } public void invalidateRelationCache(List columnNames, String schema, String table, List changedColumns, Object[] values) { - relationCacheGeneration.incrementAndGet(); - for (UniqueDataMetadata metadata : uniqueDataMetadataMap.values()) { - if (metadata.schema().equals(schema) && metadata.table().equals(table)) { - ColumnValuePair[] idColumns = new ColumnValuePair[metadata.idColumns().size()]; - for (ColumnMetadata idColumn : metadata.idColumns()) { - boolean found = false; - for (int i = 0; i < columnNames.size(); i++) { - if (idColumn.name().equals(columnNames.get(i))) { - idColumns[metadata.idColumns().indexOf(idColumn)] = new ColumnValuePair(idColumn.name(), values[i]); - found = true; - break; - } - } - Preconditions.checkArgument(found, "Not all ID columnsInReferringTable were provided for UniqueData class %s. Required: %s, Provided: %s", metadata.clazz().getName(), metadata.idColumns(), Arrays.toString(values)); - } - - ColumnValuePairs idCols = new ColumnValuePairs(idColumns); - for (String changedColumn : changedColumns) { - Cell cell = new Cell(schema, table, changedColumn, idCols); - Set queries = dependencyToRelationCacheMapping.remove(cell); - if (queries != null) { - for (SelectQuery query : queries) { - relationCache.invalidate(query); - logger.trace("Invalidated relation cache for query {} due to change in cell {}", query, cell); - } - } - } - } - } - } - - private void cleanupRelationCacheEntry(@NotNull SelectQuery query, @NotNull ReadCacheResult res) { - for (Cell dependency : res.getDependencies()) { - Set dependentQueries = dependencyToRelationCacheMapping.get(dependency); - if (dependentQueries != null) { - dependentQueries.remove(query); - if (dependentQueries.isEmpty()) { - dependencyToRelationCacheMapping.remove(dependency); - } - } - } + relationCache.invalidate(resolveCells(columnNames, schema, table, changedColumns, values)); } public @Nullable ReadCacheResult getCellCacheResult(SelectQuery query) { - return cellCache.getIfPresent(query); + return cellCache.get(query); } public long getCellCacheGeneration() { - return cellCacheGeneration.get(); + return cellCache.getGeneration(); } public void putCellCacheResult(SelectQuery query, @NotNull ReadCacheResult result, long expectedGeneration) { - if (cellCacheGeneration.get() != expectedGeneration) { - return; - } - logger.trace("Putting result in cell cache for query {} with result {}", query, result); - cellCache.put(query, result); - for (Cell cell : result.getDependencies()) { - dependencyToCellCacheMapping.computeIfAbsent(cell, k -> ConcurrentHashMap.newKeySet()) - .add(query); - } - if (cellCacheGeneration.get() != expectedGeneration) { - cellCache.invalidate(query); - cleanupCellCacheEntry(query, result); - } + cellCache.put(query, result, expectedGeneration); } public void invalidateCellCache(List columnNames, String schema, String table, List changedColumns, Object[] values) { - cellCacheGeneration.incrementAndGet(); + cellCache.invalidate(resolveCells(columnNames, schema, table, changedColumns, values)); + } + + private Set resolveCells(List columnNames, String schema, String table, List changedColumns, Object[] values) { + Set cells = new HashSet<>(); for (UniqueDataMetadata metadata : uniqueDataMetadataMap.values()) { if (metadata.schema().equals(schema) && metadata.table().equals(table)) { ColumnValuePair[] idColumns = new ColumnValuePair[metadata.idColumns().size()]; @@ -1844,28 +1786,10 @@ public void invalidateCellCache(List columnNames, String schema, String ColumnValuePairs idCols = new ColumnValuePairs(idColumns); for (String changedColumn : changedColumns) { - Cell cell = new Cell(schema, table, changedColumn, idCols); - Set queries = dependencyToCellCacheMapping.remove(cell); - if (queries != null) { - for (SelectQuery query : queries) { - cellCache.invalidate(query); - logger.trace("Invalidated cell cache for query {} due to change in cell {}", query, cell); - } - } - } - } - } - } - - private void cleanupCellCacheEntry(@NotNull SelectQuery query, @NotNull ReadCacheResult res) { - for (Cell dependency : res.getDependencies()) { - Set dependentQueries = dependencyToCellCacheMapping.get(dependency); - if (dependentQueries != null) { - dependentQueries.remove(query); - if (dependentQueries.isEmpty()) { - dependencyToCellCacheMapping.remove(dependency); + cells.add(new Cell(schema, table, changedColumn, idCols)); } } } + return cells; } } diff --git a/core/src/main/java/net/staticstudios/data/util/DependencyTrackingCache.java b/core/src/main/java/net/staticstudios/data/util/DependencyTrackingCache.java new file mode 100644 index 0000000..bec2d3a --- /dev/null +++ b/core/src/main/java/net/staticstudios/data/util/DependencyTrackingCache.java @@ -0,0 +1,97 @@ +package net.staticstudios.data.util; + +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.RemovalCause; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; + +public class DependencyTrackingCache { + private final Logger logger = LoggerFactory.getLogger(this.getClass()); + private final String name; + private final Cache cache; + private final Map> dependencyMapping = new ConcurrentHashMap<>(); + private final ReadWriteLock lock = new ReentrantReadWriteLock(); + private final AtomicLong generation = new AtomicLong(); + + public DependencyTrackingCache(String name, long maximumSize, long expireAfterWriteMinutes) { + this.name = name; + this.cache = Caffeine.newBuilder() + .maximumSize(maximumSize) + .expireAfterWrite(expireAfterWriteMinutes, TimeUnit.MINUTES) + .removalListener((SelectQuery query, ReadCacheResult result, RemovalCause cause) -> cleanup(query, result)) + .executor(Runnable::run) + .build(); + } + + public @Nullable ReadCacheResult get(SelectQuery query) { + return cache.getIfPresent(query); + } + + public long getGeneration() { + return generation.get(); + } + + public void put(SelectQuery query, @NotNull ReadCacheResult result, long expectedGeneration) { + lock.readLock().lock(); + try { + if (generation.get() != expectedGeneration) { + return; + } + logger.trace("Putting result in {} cache for query {} with result {}", name, query, result); + for (Cell cell : result.getDependencies()) { + dependencyMapping.computeIfAbsent(cell, k -> ConcurrentHashMap.newKeySet()) + .add(query); + } + cache.put(query, result); + } finally { + lock.readLock().unlock(); + } + } + + public void invalidate(Set cells) { + lock.writeLock().lock(); + try { + generation.incrementAndGet(); + for (Cell cell : cells) { + Set queries = dependencyMapping.remove(cell); + if (queries != null) { + for (SelectQuery query : queries) { + cache.invalidate(query); + logger.trace("Invalidated {} cache for query {} due to change in cell {}", name, query, cell); + } + } + } + } finally { + lock.writeLock().unlock(); + } + } + + public int estimatedSize() { + return (int) cache.estimatedSize(); + } + + public int dependencyMappingSize() { + return dependencyMapping.size(); + } + + private void cleanup(@NotNull SelectQuery query, @NotNull ReadCacheResult res) { + for (Cell dependency : res.getDependencies()) { + dependencyMapping.computeIfPresent(dependency, (k, dependentQueries) -> { + dependentQueries.remove(query); + return dependentQueries.isEmpty() ? null : dependentQueries; + }); + } + } +} + From 55159f01c22b96773111cbf115e80644a1e80cd6 Mon Sep 17 00:00:00 2001 From: Sammy Aknan Date: Fri, 13 Mar 2026 16:51:40 -0400 Subject: [PATCH 2/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../data/util/DependencyTrackingCache.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/net/staticstudios/data/util/DependencyTrackingCache.java b/core/src/main/java/net/staticstudios/data/util/DependencyTrackingCache.java index bec2d3a..34b6545 100644 --- a/core/src/main/java/net/staticstudios/data/util/DependencyTrackingCache.java +++ b/core/src/main/java/net/staticstudios/data/util/DependencyTrackingCache.java @@ -29,7 +29,11 @@ public DependencyTrackingCache(String name, long maximumSize, long expireAfterWr this.cache = Caffeine.newBuilder() .maximumSize(maximumSize) .expireAfterWrite(expireAfterWriteMinutes, TimeUnit.MINUTES) - .removalListener((SelectQuery query, ReadCacheResult result, RemovalCause cause) -> cleanup(query, result)) + .removalListener((SelectQuery query, ReadCacheResult result, RemovalCause cause) -> { + if (query != null && result != null && cause != RemovalCause.REPLACED) { + cleanup(query, result); + } + }) .executor(Runnable::run) .build(); } @@ -49,6 +53,13 @@ public void put(SelectQuery query, @NotNull ReadCacheResult result, long expecte return; } logger.trace("Putting result in {} cache for query {} with result {}", name, query, result); + + // If there's an existing cached result for this query, clean up its dependencies first + ReadCacheResult previous = cache.getIfPresent(query); + if (previous != null) { + cleanup(query, previous); + } + for (Cell cell : result.getDependencies()) { dependencyMapping.computeIfAbsent(cell, k -> ConcurrentHashMap.newKeySet()) .add(query); From 540494361ec263e35446d9148e5eaa63d849b42d Mon Sep 17 00:00:00 2001 From: Sammy Aknan Date: Fri, 13 Mar 2026 16:52:14 -0400 Subject: [PATCH 3/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../data/util/DependencyTrackingCache.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/net/staticstudios/data/util/DependencyTrackingCache.java b/core/src/main/java/net/staticstudios/data/util/DependencyTrackingCache.java index 34b6545..bd36210 100644 --- a/core/src/main/java/net/staticstudios/data/util/DependencyTrackingCache.java +++ b/core/src/main/java/net/staticstudios/data/util/DependencyTrackingCache.java @@ -97,11 +97,16 @@ public int dependencyMappingSize() { } private void cleanup(@NotNull SelectQuery query, @NotNull ReadCacheResult res) { - for (Cell dependency : res.getDependencies()) { - dependencyMapping.computeIfPresent(dependency, (k, dependentQueries) -> { - dependentQueries.remove(query); - return dependentQueries.isEmpty() ? null : dependentQueries; - }); + lock.readLock().lock(); + try { + for (Cell dependency : res.getDependencies()) { + dependencyMapping.computeIfPresent(dependency, (k, dependentQueries) -> { + dependentQueries.remove(query); + return dependentQueries.isEmpty() ? null : dependentQueries; + }); + } + } finally { + lock.readLock().unlock(); } } } From a9499ddd3dc43e47f2cc543a9704a6298123d739 Mon Sep 17 00:00:00 2001 From: Sammy Aknan Date: Fri, 13 Mar 2026 16:57:49 -0400 Subject: [PATCH 4/6] changes --- .../util/DependencyTrackingCacheTest.java | 325 ++++++++++++++++++ 1 file changed, 325 insertions(+) create mode 100644 core/src/test/java/net/staticstudios/data/util/DependencyTrackingCacheTest.java diff --git a/core/src/test/java/net/staticstudios/data/util/DependencyTrackingCacheTest.java b/core/src/test/java/net/staticstudios/data/util/DependencyTrackingCacheTest.java new file mode 100644 index 0000000..4fa319a --- /dev/null +++ b/core/src/test/java/net/staticstudios/data/util/DependencyTrackingCacheTest.java @@ -0,0 +1,325 @@ +package net.staticstudios.data.util; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.*; + +class DependencyTrackingCacheTest { + + private DependencyTrackingCache cache; + + private static final String SCHEMA = "public"; + private static final String TABLE = "users"; + + @BeforeEach + void setUp() { + cache = new DependencyTrackingCache("test", 1000, 60); + } + + private SelectQuery query(String tag) { + return new SelectQuery(tag, "SELECT * FROM users WHERE id = ?", List.of(1)); + } + + private Cell cell(String column, Object idValue) { + ColumnValuePairs ids = new ColumnValuePairs(new ColumnValuePair("id", idValue)); + return new Cell(SCHEMA, TABLE, column, ids); + } + + private ReadCacheResult result(Object value, Cell... dependencies) { + return new ReadCacheResult(value, Set.of(dependencies)); + } + + // --- Basic put / get --- + + @Test + void putAndGet() { + long gen = cache.getGeneration(); + SelectQuery q = query("basic"); + Cell c = cell("name", 1); + ReadCacheResult r = result("Alice", c); + + cache.put(q, r, gen); + + ReadCacheResult cached = cache.get(q); + assertNotNull(cached); + assertEquals("Alice", cached.getValue()); + assertEquals(1, cache.estimatedSize()); + assertEquals(1, cache.dependencyMappingSize()); + } + + @Test + void getMissReturnsNull() { + assertNull(cache.get(query("missing"))); + } + + // --- Generation gating --- + + @Test + void putRejectedWhenGenerationMismatch() { + long staleGen = cache.getGeneration(); + Cell c = cell("name", 1); + + cache.invalidate(Set.of(c)); + + cache.put(query("stale"), result("stale-value", c), staleGen); + + assertNull(cache.get(query("stale"))); + assertEquals(0, cache.estimatedSize()); + } + + @Test + void putAcceptedWhenGenerationMatches() { + Cell c = cell("name", 1); + cache.invalidate(Set.of(c)); + + long currentGen = cache.getGeneration(); + SelectQuery q = query("fresh"); + cache.put(q, result("fresh-value", c), currentGen); + + assertNotNull(cache.get(q)); + } + + // --- Invalidation removes cache entries and dependency mappings --- + + @Test + void invalidateRemovesEntryAndDependencies() { + long gen = cache.getGeneration(); + Cell c = cell("name", 1); + SelectQuery q = query("inv"); + + cache.put(q, result("value", c), gen); + assertEquals(1, cache.dependencyMappingSize()); + + cache.invalidate(Set.of(c)); + + assertNull(cache.get(q)); + assertEquals(0, cache.dependencyMappingSize()); + } + + @Test + void invalidateAcrossMultipleCells() { + long gen = cache.getGeneration(); + Cell c1 = cell("name", 1); + Cell c2 = cell("email", 1); + SelectQuery q = query("multi-dep"); + + cache.put(q, result("value", c1, c2), gen); + assertEquals(2, cache.dependencyMappingSize()); + + cache.invalidate(Set.of(c1)); + + assertNull(cache.get(q)); + assertEquals(0, cache.dependencyMappingSize()); + } + + @Test + void invalidateOnlyAffectsRelatedQueries() { + long gen = cache.getGeneration(); + Cell c1 = cell("name", 1); + Cell c2 = cell("name", 2); + SelectQuery q1 = query("user1"); + SelectQuery q2 = new SelectQuery("user2", "SELECT * FROM users WHERE id = ?", List.of(2)); + + cache.put(q1, result("Alice", c1), gen); + cache.put(q2, result("Bob", c2), gen); + + cache.invalidate(Set.of(c1)); + + assertNull(cache.get(q1)); + assertNotNull(cache.get(q2)); + } + + // --- Replacement (re-put same SelectQuery) --- + + @Test + void replacementCleansUpOldDependencies() { + long gen = cache.getGeneration(); + Cell oldDep = cell("name", 1); + Cell newDep = cell("email", 1); + SelectQuery q = query("replace"); + + cache.put(q, result("old", oldDep), gen); + assertEquals(1, cache.dependencyMappingSize()); + + cache.put(q, result("new", newDep), gen); + assertEquals(1, cache.dependencyMappingSize()); + assertEquals("new", cache.get(q).getValue()); + + cache.invalidate(Set.of(oldDep)); + assertNotNull(cache.get(q)); + } + + @Test + void replacementRegistersNewDependencies() { + long gen = cache.getGeneration(); + Cell oldDep = cell("name", 1); + Cell newDep = cell("email", 1); + SelectQuery q = query("replace2"); + + cache.put(q, result("old", oldDep), gen); + cache.put(q, result("new", newDep), gen); + + cache.invalidate(Set.of(newDep)); + assertNull(cache.get(q)); + } + + @Test + void replacementWithOverlappingDependencies() { + long gen = cache.getGeneration(); + Cell shared = cell("id", 1); + Cell onlyOld = cell("name", 1); + Cell onlyNew = cell("email", 1); + SelectQuery q = query("overlap"); + + cache.put(q, result("old", shared, onlyOld), gen); + assertEquals(2, cache.dependencyMappingSize()); + + cache.put(q, result("new", shared, onlyNew), gen); + assertEquals(2, cache.dependencyMappingSize()); + + cache.invalidate(Set.of(onlyOld)); + assertNotNull(cache.get(q)); + + long gen2 = cache.getGeneration(); + cache.put(q, result("new2", shared, onlyNew), gen2); + + cache.invalidate(Set.of(onlyNew)); + assertNull(cache.get(q)); + } + + // --- Generation counter increments on each invalidation --- + + @Test + void generationIncrementsOnInvalidation() { + long gen0 = cache.getGeneration(); + cache.invalidate(Set.of(cell("a", 1))); + long gen1 = cache.getGeneration(); + cache.invalidate(Set.of(cell("b", 2))); + long gen2 = cache.getGeneration(); + + assertEquals(gen0 + 1, gen1); + assertEquals(gen1 + 1, gen2); + } + + // --- Multiple queries sharing the same cell dependency --- + + @Test + void multipleDependentsOnSameCell() { + long gen = cache.getGeneration(); + Cell shared = cell("name", 1); + SelectQuery q1 = query("q1"); + SelectQuery q2 = new SelectQuery("q2", "SELECT name FROM users WHERE id = ?", List.of(1)); + + cache.put(q1, result("v1", shared), gen); + cache.put(q2, result("v2", shared), gen); + assertEquals(1, cache.dependencyMappingSize()); + + cache.invalidate(Set.of(shared)); + + assertNull(cache.get(q1)); + assertNull(cache.get(q2)); + assertEquals(0, cache.dependencyMappingSize()); + } + + // --- Empty dependency set --- + + @Test + void putWithNoDependencies() { + long gen = cache.getGeneration(); + SelectQuery q = query("no-deps"); + cache.put(q, result("value"), gen); + + assertNotNull(cache.get(q)); + assertEquals(0, cache.dependencyMappingSize()); + } + + // --- Invalidating cells with no dependents is a no-op --- + + @Test + void invalidateUnrelatedCellIsNoOp() { + long gen = cache.getGeneration(); + Cell c = cell("name", 1); + Cell unrelated = cell("name", 999); + SelectQuery q = query("safe"); + + cache.put(q, result("value", c), gen); + cache.invalidate(Set.of(unrelated)); + + assertNotNull(cache.get(q)); + } + + // --- Concurrent put vs invalidate --- + + @Test + void concurrentPutAndInvalidateDoNotCorrupt() throws Exception { + int iterations = 500; + int threads = 8; + ExecutorService executor = Executors.newFixedThreadPool(threads); + AtomicInteger corruptionCount = new AtomicInteger(); + + for (int i = 0; i < iterations; i++) { + Cell c = cell("name", i); + CyclicBarrier barrier = new CyclicBarrier(2); + CountDownLatch done = new CountDownLatch(2); + + int idx = i; + executor.submit(() -> { + try { + barrier.await(); + long gen = cache.getGeneration(); + SelectQuery q = new SelectQuery("c", "SELECT * FROM t WHERE id = ?", List.of(idx)); + cache.put(q, result("v", c), gen); + } catch (Exception e) { + corruptionCount.incrementAndGet(); + } finally { + done.countDown(); + } + }); + + executor.submit(() -> { + try { + barrier.await(); + cache.invalidate(Set.of(c)); + } catch (Exception e) { + corruptionCount.incrementAndGet(); + } finally { + done.countDown(); + } + }); + + done.await(); + } + + executor.shutdown(); + assertEquals(0, corruptionCount.get()); + + assertTrue(cache.dependencyMappingSize() >= 0); + assertTrue(cache.estimatedSize() >= 0); + } + + // --- Stale generation from concurrent invalidation prevents caching stale data --- + + @Test + void staleDataNotCachedAfterConcurrentInvalidation() throws Exception { + Cell c = cell("name", 1); + SelectQuery q = query("stale-race"); + + long genBeforeInvalidation = cache.getGeneration(); + + cache.invalidate(Set.of(c)); + + cache.put(q, result("stale", c), genBeforeInvalidation); + + assertNull(cache.get(q)); + } +} + From 491d64b60e8807c2a9a4713a315871fbbc28710b Mon Sep 17 00:00:00 2001 From: Sammy Aknan Date: Fri, 13 Mar 2026 17:02:35 -0400 Subject: [PATCH 5/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../util/DependencyTrackingCacheTest.java | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/core/src/test/java/net/staticstudios/data/util/DependencyTrackingCacheTest.java b/core/src/test/java/net/staticstudios/data/util/DependencyTrackingCacheTest.java index 4fa319a..f0ef2d5 100644 --- a/core/src/test/java/net/staticstudios/data/util/DependencyTrackingCacheTest.java +++ b/core/src/test/java/net/staticstudios/data/util/DependencyTrackingCacheTest.java @@ -321,5 +321,55 @@ void staleDataNotCachedAfterConcurrentInvalidation() throws Exception { assertNull(cache.get(q)); } + + @Test + void concurrentReputSameQueryDoesNotCorrupt() throws Exception { + int iterations = 500; + ExecutorService executor = Executors.newFixedThreadPool(2); + AtomicInteger corruptionCount = new AtomicInteger(); + + // Use a single shared query key to exercise concurrent re-put/replacement behavior. + SelectQuery q = query("concurrent-reput"); + + for (int i = 0; i < iterations; i++) { + CyclicBarrier barrier = new CyclicBarrier(2); + CountDownLatch done = new CountDownLatch(2); + int idx = i; + + executor.submit(() -> { + try { + barrier.await(); + long gen = cache.getGeneration(); + Cell cA = cell("name-a", idx); + cache.put(q, result("v-a", cA), gen); + } catch (Exception e) { + corruptionCount.incrementAndGet(); + } finally { + done.countDown(); + } + }); + + executor.submit(() -> { + try { + barrier.await(); + long gen = cache.getGeneration(); + Cell cB = cell("name-b", idx); + cache.put(q, result("v-b", cB), gen); + } catch (Exception e) { + corruptionCount.incrementAndGet(); + } finally { + done.countDown(); + } + }); + + done.await(); + } + + executor.shutdown(); + + assertEquals(0, corruptionCount.get()); + assertTrue(cache.dependencyMappingSize() >= 0); + assertTrue(cache.estimatedSize() >= 0); + } } From ac75156fd67c407b89f39107e337486158bd677c Mon Sep 17 00:00:00 2001 From: Sammy Aknan Date: Fri, 13 Mar 2026 17:02:43 -0400 Subject: [PATCH 6/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../staticstudios/data/util/DependencyTrackingCache.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/net/staticstudios/data/util/DependencyTrackingCache.java b/core/src/main/java/net/staticstudios/data/util/DependencyTrackingCache.java index bd36210..8ecd850 100644 --- a/core/src/main/java/net/staticstudios/data/util/DependencyTrackingCache.java +++ b/core/src/main/java/net/staticstudios/data/util/DependencyTrackingCache.java @@ -47,7 +47,7 @@ public long getGeneration() { } public void put(SelectQuery query, @NotNull ReadCacheResult result, long expectedGeneration) { - lock.readLock().lock(); + lock.writeLock().lock(); try { if (generation.get() != expectedGeneration) { return; @@ -66,7 +66,7 @@ public void put(SelectQuery query, @NotNull ReadCacheResult result, long expecte } cache.put(query, result); } finally { - lock.readLock().unlock(); + lock.writeLock().unlock(); } } @@ -97,7 +97,7 @@ public int dependencyMappingSize() { } private void cleanup(@NotNull SelectQuery query, @NotNull ReadCacheResult res) { - lock.readLock().lock(); + lock.writeLock().lock(); try { for (Cell dependency : res.getDependencies()) { dependencyMapping.computeIfPresent(dependency, (k, dependentQueries) -> { @@ -106,7 +106,7 @@ private void cleanup(@NotNull SelectQuery query, @NotNull ReadCacheResult res) { }); } } finally { - lock.readLock().unlock(); + lock.writeLock().unlock(); } } }