Skip to content

Conversation

@ngngwr
Copy link
Collaborator

@ngngwr ngngwr commented Dec 19, 2025

Issues

  • My PR addresses the following Helix issues and references them in the PR description:

Fixes race condition in parallel resource computation introduced by BestPossibleState calc parallelization

Description

  • Here are some details about my PR, including screenshots of any UI changes:
    What:
    Fix thread-safety issue in BestPossibleStateOutput class that caused intermittent test failures when resources are computed in parallel.
    Why:
    The _preferenceLists field in BestPossibleStateOutput used lazy initialization with a check-then-act pattern that was not thread-safe:

// BEFORE: Race condition - multiple threads could pass the null check// and overwrite each other's ConcurrentHashMap instanceif (_preferenceLists == null) { _preferenceLists = new ConcurrentHashMap<>();}_preferenceLists.put(resource, resourcePreferenceLists);

When BestPossibleStateCalcStage computes resources in parallel using StageThreadPoolHelper, multiple threads could simultaneously:

  • Check _preferenceLists == null → both return true
  • Create new ConcurrentHashMap instances
  • One thread's map overwrites the other's, losing resource data

How:
Changed _preferenceLists from lazy initialization to eager initialization:
`

// AFTER: Thread-safe - field is initialized once at declarationprivate final Map<String, Map<String, List>> _preferenceLists = new ConcurrentHashMap<>();

`
Key changes:

  • Initialize _preferenceLists eagerly at field declaration
  • Make the field final to prevent reassignment
  • Remove all null checks from getter/setter methods
  • Simplify method implementations

Tests

No new tests added. This fix resolves existing failing tests:

TestBestPossibleStateCalcStage.testBestPossibleComputationWithMultipleResources

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