Skip to content

fix: make MemoryUsageRegister thread-safe for concurrent validations#2169

Merged
davidgamez merged 1 commit into
MobilityData:masterfrom
JarvusInnovations:fix/2168-memory-register-thread-safety
Jun 25, 2026
Merged

fix: make MemoryUsageRegister thread-safe for concurrent validations#2169
davidgamez merged 1 commit into
MobilityData:masterfrom
JarvusInnovations:fix/2168-memory-register-thread-safety

Conversation

@themightychris

Copy link
Copy Markdown
Collaborator

Summary

MemoryUsageRegister is a process-global singleton — a single static instance with an unsynchronized registry list — and ValidationRunner.run() calls clearRegistry() at the start of every run. When two validations run concurrently in the same JVM, they append to the same list and one run's clear() wipes the other's in-flight snapshots, so summary.memoryUsageRecords is corrupted. Validation notices are unaffected, since the NoticeContainer is per-run.

This is the bug described in #2168.

Fix

Thread-confine the register. Both @MemoryMonitor interception points — ValidationRunner.run and GtfsFeedLoader.loadAndValidate — execute on the thread that invokes run(), so giving each thread its own register isolates concurrent validations with no locking and no behavior change for the single-threaded CLI/desktop path.

private static final ThreadLocal<MemoryUsageRegister> INSTANCE =
    ThreadLocal.withInitial(MemoryUsageRegister::new);

public static MemoryUsageRegister getInstance() {
  return INSTANCE.get();
}

Test

MemoryUsageRegisterTest adds a barrier-driven interleaving that forces the exact corruption (thread B calls clearRegistry() after thread A has registered a snapshot). On the old singleton it fails with expected: A1,A2 but was: B1,A2; with the fix each thread sees only its own snapshots.

Validation against a real concurrent service

We exercised this against an HTTP validator service (a Spring wrapper around a shared ValidationRunner, as in MobilityData/gtfs-validator-api#1), running 8 concurrent validations of a ~3 MB / ~13k-trip feed:

  • Before: the 8 runs reported 4 / 13 / 14 / 17 / 24 / 28 / 28 / 31 / 32 memoryUsageRecords (the single-run baseline is 4) — cross-contaminated.
  • After: all 8 runs report exactly 4 records with no cross-run contamination; notices are identical across runs.

Resolves #2168.

MemoryUsageRegister is a process-global singleton with an unsynchronized
registry list and a clearRegistry() that ValidationRunner.run() calls at the
start of every run. When two validations overlap they append to the same list
and one run's clear() wipes the other's in-flight snapshots, corrupting
summary.memoryUsageRecords. Validation notices are unaffected because the
NoticeContainer is per-run.

Both @MemoryMonitor interception points (ValidationRunner.run and
GtfsFeedLoader.loadAndValidate) execute on the request thread, so
thread-confining the register isolates concurrent runs with no locking and no
behavior change for the single-threaded CLI/desktop path.

Adds MemoryUsageRegisterTest with a barrier-driven interleaving that reproduces
the corruption (expected: A1,A2 but was: B1,A2 on the old singleton) and passes
with the fix.

Resolves MobilityData#2168.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C1uYxGwNbaGQk9MUajBms4

@davidgamez davidgamez left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@davidgamez davidgamez merged commit bd5120b into MobilityData:master Jun 25, 2026
103 checks passed
@themightychris themightychris deleted the fix/2168-memory-register-thread-safety branch June 25, 2026 19:56
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.

MemoryUsageRegister not thread safe

2 participants