Skip to content

Extract concurrency utilities, add multithreaded tests, fix two races#30

Merged
predatorray merged 1 commit into
mainfrom
claude/concurrency-test-coverage-1d1aew
Jun 20, 2026
Merged

Extract concurrency utilities, add multithreaded tests, fix two races#30
predatorray merged 1 commit into
mainfrom
claude/concurrency-test-coverage-1d1aew

Conversation

@predatorray

Copy link
Copy Markdown
Owner

Pull the hand-rolled "complicated combinations" of concurrency primitives
that were duplicated across modules into small, business-logic-free,
thoroughly multithreaded-tested utilities in a new common.concurrent
package (and a CoordinationCas helper in the coordination module), then
swap the call sites onto them. Along the way, fix two latent concurrency
defects surfaced by the audit.

New utilities (each with race-injecting concurrent tests):

  • BoundedLruCache: thread-safe access-ordered LRU (was an inline
    synchronizedMap(LinkedHashMap{removeEldestEntry}) in BoxEngine).
  • TtlCache: read-through TTL cache with loaders run outside the locks
    (was a ConcurrentHashMap<K,(value,expiresAt)> copy in BoxAclStore,
    S3AccessControl, and ClusterRouter).
  • KeyedSlidingWindow: per-key bounded sliding window (was a
    ConcurrentHashMap<K,ArrayDeque> guarded by synchronized(deque) in
    MetricsScraper).
  • PeriodicGate: run-at-most-once-per-interval throttle via a volatile
    deadline + double-checked lock (was inline in FileCredentialStore).
  • CoordinationCas: optimistic upsert/delete-with-bounded-retry over
    CoordinationService (was duplicated in BoxAclStore and CandyboxNode).

Defect fixes (each with a failing-first regression test):

  • ClusterRouter opened connections inside ConcurrentHashMap.computeIfAbsent,
    running blocking connect() under a bin lock (a documented anti-pattern).
    Connect outside the map locks now; a lost race closes the surplus.
  • BoxEngine checked the idempotency cache only outside the write lock, so
    two concurrent retries of the same token could both apply the mutation.
    Re-check under the lock so the second caller returns the cached result.

All affected module unit suites pass (583 tests, 0 failures); both
regression tests were confirmed to fail when the fix is reverted.

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01CdFZHjRQrHzD6FgbDgzh1B

Pull the hand-rolled "complicated combinations" of concurrency primitives
that were duplicated across modules into small, business-logic-free,
thoroughly multithreaded-tested utilities in a new common.concurrent
package (and a CoordinationCas helper in the coordination module), then
swap the call sites onto them. Along the way, fix two latent concurrency
defects surfaced by the audit.

New utilities (each with race-injecting concurrent tests):
- BoundedLruCache: thread-safe access-ordered LRU (was an inline
  synchronizedMap(LinkedHashMap{removeEldestEntry}) in BoxEngine).
- TtlCache: read-through TTL cache with loaders run outside the locks
  (was a ConcurrentHashMap<K,(value,expiresAt)> copy in BoxAclStore,
  S3AccessControl, and ClusterRouter).
- KeyedSlidingWindow: per-key bounded sliding window (was a
  ConcurrentHashMap<K,ArrayDeque> guarded by synchronized(deque) in
  MetricsScraper).
- PeriodicGate: run-at-most-once-per-interval throttle via a volatile
  deadline + double-checked lock (was inline in FileCredentialStore).
- CoordinationCas: optimistic upsert/delete-with-bounded-retry over
  CoordinationService (was duplicated in BoxAclStore and CandyboxNode).

Defect fixes (each with a failing-first regression test):
- ClusterRouter opened connections inside ConcurrentHashMap.computeIfAbsent,
  running blocking connect() under a bin lock (a documented anti-pattern).
  Connect outside the map locks now; a lost race closes the surplus.
- BoxEngine checked the idempotency cache only outside the write lock, so
  two concurrent retries of the same token could both apply the mutation.
  Re-check under the lock so the second caller returns the cached result.

All affected module unit suites pass (583 tests, 0 failures); both
regression tests were confirmed to fail when the fix is reverted.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CdFZHjRQrHzD6FgbDgzh1B
@github-actions

Copy link
Copy Markdown

✅ S3 compatibility gate passed

  • mode: gate
  • result: 192 passed in 87.77s (0:01:27)

@codecov

codecov Bot commented Jun 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.57447% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.82%. Comparing base (4bf1f55) to head (c447245).

Files with missing lines Patch % Lines
...edatorray/candybox/common/concurrent/TtlCache.java 81.25% 4 Missing and 2 partials ⚠️
.../me/predatorray/candybox/lsm/engine/BoxEngine.java 76.92% 3 Missing and 3 partials ⚠️
...ay/candybox/common/concurrent/BoundedLruCache.java 76.47% 4 Missing ⚠️
...va/me/predatorray/candybox/server/BoxAclStore.java 66.66% 4 Missing ⚠️
...candybox/common/concurrent/KeyedSlidingWindow.java 89.65% 3 Missing ⚠️
...orray/candybox/common/concurrent/PeriodicGate.java 87.50% 1 Missing and 1 partial ⚠️
...atorray/candybox/coordination/CoordinationCas.java 91.30% 1 Missing and 1 partial ⚠️
.../me/predatorray/candybox/client/ClusterRouter.java 94.73% 1 Missing ⚠️
...a/me/predatorray/candybox/server/CandyboxNode.java 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               main      #30   +/-   ##
=========================================
  Coverage     83.82%   83.82%           
+ Complexity      677      674    -3     
=========================================
  Files           162      167    +5     
  Lines          9284     9369   +85     
  Branches       1397     1404    +7     
=========================================
+ Hits           7782     7854   +72     
- Misses          998     1012   +14     
+ Partials        504      503    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@predatorray predatorray merged commit 3e27ba0 into main Jun 20, 2026
9 checks passed
@predatorray predatorray deleted the claude/concurrency-test-coverage-1d1aew branch June 20, 2026 04:49
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.

2 participants