Skip to content

fix: in event consumption, always release the lock if we already aquired it#2169

Merged
triceo merged 2 commits intoTimefoldAI:1.xfrom
triceo:threads-1.x
Mar 6, 2026
Merged

fix: in event consumption, always release the lock if we already aquired it#2169
triceo merged 2 commits intoTimefoldAI:1.xfrom
triceo:threads-1.x

Conversation

@triceo
Copy link
Collaborator

@triceo triceo commented Mar 6, 2026

Cherry-pick of 5065095

Copilot AI review requested due to automatic review settings March 6, 2026 07:58
@triceo triceo self-assigned this Mar 6, 2026
@triceo triceo added this to the v1.32.0 (Maintenance release) milestone Mar 6, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a concurrency bug in the Timefold Solver's event consumption mechanism where semaphore locks (used to order solver events) were not always properly released. The fix moves lock release logic into whenComplete/whenCompleteAsync callbacks on CompletableFuture, ensuring locks are released even when exceptions occur. Additionally, mutable fields accessed across threads are converted to AtomicReference for proper thread safety, and a timeout is added to terminateEarly() to prevent indefinite blocking.

Changes:

  • Refactored ConsumerSupport to release semaphores in whenComplete callbacks instead of finally blocks inside async tasks, ensuring lock release even on exceptional paths.
  • Converted DefaultSolverJob fields (solverStatus, finalBestSolutionFuture, consumerSupport) from volatile/plain fields to AtomicReference for proper thread-safe access.
  • Added a 1-minute timeout to terminateEarly() with escalation logic to prevent indefinite blocking.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolverJob.java Converted fields to AtomicReference, extracted terminateEarlyLocked() with timeout and idempotency, added null-safety checks
core/src/main/java/ai/timefold/solver/core/impl/solver/ConsumerSupport.java Moved semaphore releases to whenComplete callbacks, added @NullMarked, added class-level Javadoc, refactored to return CompletableFuture for chaining
core/src/main/java/ai/timefold/solver/core/impl/solver/BestSolutionHolder.java Removed redundant @NonNull annotation (class is already @NullMarked)
core/src/main/java/ai/timefold/solver/core/api/solver/SolverJob.java Updated Javadoc for terminateEarly() to document the new timeout behavior

@triceo triceo merged commit 13bf59d into TimefoldAI:1.x Mar 6, 2026
20 checks passed
@triceo triceo deleted the threads-1.x branch March 6, 2026 13:57
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.

3 participants