diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/performance/MemoryUsageRegister.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/performance/MemoryUsageRegister.java index d3be3c2762..b0a42cf60f 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/performance/MemoryUsageRegister.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/performance/MemoryUsageRegister.java @@ -9,7 +9,13 @@ public class MemoryUsageRegister { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private static MemoryUsageRegister instance = new MemoryUsageRegister(); + // Thread-confined register: a validation runs entirely on the single thread + // that invokes ValidationRunner.run (the @MemoryMonitor-annotated entry points + // and the manual snapshots all execute on it), so giving each thread its own + // register isolates concurrent validations with no locking and no cross-run + // clearRegistry()/append contamination. Fixes #2168. + private static final ThreadLocal INSTANCE = + ThreadLocal.withInitial(MemoryUsageRegister::new); private final Runtime runtime; private List registry = new ArrayList<>(); @@ -18,10 +24,11 @@ private MemoryUsageRegister() { } /** - * @return the singleton instance of the memory usage register. + * @return the memory usage register for the calling thread. Each thread gets its own register so + * that concurrent validations do not share state. */ public static MemoryUsageRegister getInstance() { - return instance; + return INSTANCE.get(); } /** diff --git a/core/src/test/java/org/mobilitydata/gtfsvalidator/performance/MemoryUsageRegisterTest.java b/core/src/test/java/org/mobilitydata/gtfsvalidator/performance/MemoryUsageRegisterTest.java new file mode 100644 index 0000000000..9c1e028c83 --- /dev/null +++ b/core/src/test/java/org/mobilitydata/gtfsvalidator/performance/MemoryUsageRegisterTest.java @@ -0,0 +1,124 @@ +/* + * Copyright 2026 MobilityData + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.mobilitydata.gtfsvalidator.performance; + +import static com.google.common.truth.Truth.assertThat; + +import java.util.List; +import java.util.concurrent.CyclicBarrier; +import java.util.stream.Collectors; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class MemoryUsageRegisterTest { + + @Test + public void getInstance_isThreadConfined() { + assertThat(MemoryUsageRegister.getInstance()) + .isSameInstanceAs(MemoryUsageRegister.getInstance()); + + MemoryUsageRegister[] otherThreadInstance = new MemoryUsageRegister[1]; + Thread other = new Thread(() -> otherThreadInstance[0] = MemoryUsageRegister.getInstance()); + other.start(); + join(other); + + assertThat(otherThreadInstance[0]).isNotSameInstanceAs(MemoryUsageRegister.getInstance()); + } + + @Test + public void singleThread_registersAndClears() { + MemoryUsageRegister register = MemoryUsageRegister.getInstance(); + register.clearRegistry(); + register.registerMemoryUsage("a"); + register.registerMemoryUsage("b"); + + assertThat(keysOf(register)).containsExactly("a", "b").inOrder(); + + register.clearRegistry(); + assertThat(register.getRegistry()).isEmpty(); + } + + /** + * Two concurrent validations must each see only their own snapshots. The barriers force the exact + * interleaving that corrupts a shared singleton: thread B calls {@code clearRegistry()} after + * thread A has already registered a snapshot. With a process-global register, A's snapshot is + * wiped and B's leaks into A's report; with a thread-confined register, each thread's report is + * intact. Reproduces and guards against #2168. + */ + @Test + public void concurrentValidations_doNotShareRegistry() throws Exception { + CyclicBarrier phase = new CyclicBarrier(2); + String[] keysA = new String[1]; + String[] keysB = new String[1]; + + Thread a = + new Thread( + () -> { + MemoryUsageRegister register = MemoryUsageRegister.getInstance(); + register.clearRegistry(); + register.registerMemoryUsage("A1"); + await(phase); // P1: A has registered A1 + await(phase); // P2: let B clear + register B1 + register.registerMemoryUsage("A2"); + await(phase); // P3: reports are final + keysA[0] = String.join(",", keysOf(register)); + }); + + Thread b = + new Thread( + () -> { + MemoryUsageRegister register = MemoryUsageRegister.getInstance(); + await(phase); // P1 + register.clearRegistry(); + register.registerMemoryUsage("B1"); + await(phase); // P2 + await(phase); // P3 + keysB[0] = String.join(",", keysOf(register)); + }); + + a.start(); + b.start(); + join(a); + join(b); + + assertThat(keysA[0]).isEqualTo("A1,A2"); + assertThat(keysB[0]).isEqualTo("B1"); + } + + private static List keysOf(MemoryUsageRegister register) { + return register.getRegistry().stream().map(MemoryUsage::getKey).collect(Collectors.toList()); + } + + private static void await(CyclicBarrier barrier) { + try { + barrier.await(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + private static void join(Thread thread) { + try { + thread.join(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException(e); + } + } +}