Skip to content

Conversation

@lauraneto
Copy link
Contributor

Summary

  • Use EagerWriteLock instead of WriteLock for long running operations lock acquisition

Details

This change ensures proper lock acquisition timing when checking if a long running operation of the same type is already running. With the introduction of lazy locks in #21102, this lock needs to be explicitly eager to maintain the expected behavior.

Switch from WriteLock to EagerWriteLock when acquiring the lock for
long running operations to ensure proper lock acquisition timing.
Copilot AI review requested due to automatic review settings December 10, 2025 13:48
Copy link
Contributor

@nikolajlauridsen nikolajlauridsen left a comment

Choose a reason for hiding this comment

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

Looks good 👍

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 potential race condition in the long-running operation service by changing from lazy lock acquisition (WriteLock) to eager lock acquisition (EagerWriteLock). Following the introduction of lazy locks in #21102, locks requested via WriteLock are queued and only acquired later when needed (e.g., when database access occurs). However, the long-running operation service needs to immediately hold the lock before checking if an operation of the same type is already running to prevent concurrent operations from bypassing this check.

Key Changes

  • Changed scope.WriteLock() to scope.EagerWriteLock() in LongRunningOperationService to ensure immediate lock acquisition before the IsAlreadyRunning check

@nikolajlauridsen nikolajlauridsen merged commit 8c187a6 into main Dec 10, 2025
32 of 33 checks passed
@nikolajlauridsen nikolajlauridsen deleted the v17/bugfix/long-running-operation-eager-write-lock branch December 10, 2025 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants