Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhanced unlock() method of JdbcLock to verify successful unlocking #9291

Closed
EddieChoCho opened this issue Jun 30, 2024 · 0 comments · Fixed by #9292
Closed

Enhanced unlock() method of JdbcLock to verify successful unlocking #9291

EddieChoCho opened this issue Jun 30, 2024 · 0 comments · Fixed by #9292

Comments

@EddieChoCho
Copy link
Contributor

EddieChoCho commented Jun 30, 2024

Expected Behavior

  • The unlock() method of JdbcLock should verify the execution result of releasing the lock ownership.
  • If the ownership can not be removed due to data expiration, a ConcurrentModificationException should be thrown.
  • The RedisLock implementation should also throw a ConcurrentModificationException in similar scenarios.

Current Behavior

  • The unlock() method does not verify the execution result of releasing the lock ownership.
  • The RedisLock throws an IllegalStateException when attempting to release the lock after the ownership has expired.

Context

If the unlock() method does not verify the execution result of releasing the lock ownership, users might face concurrency issues without being notified. For example:

  1. Process A and process B are operating in the same region. Process A acquires the distributed lock with lock_key: 'key'.
  2. Due to prolonged work, the ownership of the lock held by process A expires.
  3. Before process A releases the lock, process B also acquires the distributed lock with same lock_key.
  4. When process A attempts to release the distributed lock, the current implementation does not inform the user that the integrity of data protected by this lock may have been compromised.

To address this, it is proposed that the unlock() method be enhanced to verify if the ownership of lock has been removed successfully. If the ownership cannot be removed due to data expiration, a ConcurrentModificationException will be thrown. This approach maintains the integrity of the distributed lock mechanism.

@EddieChoCho EddieChoCho added status: waiting-for-triage The issue need to be evaluated and its future decided type: enhancement labels Jun 30, 2024
EddieChoCho added a commit to EddieChoCho/spring-integration that referenced this issue Jun 30, 2024
…y successful unlocking

Fixes: spring-projects#9291
* Modify `unlock()` method of `JdbcLock`: if the lock ownership can not be removed due to data expiration, a `ConcurrentModificationException` should be thrown.
* Modify `unlock()` method of `RedisLock`: if the lock ownership can not be removed due to data expiration, a `ConcurrentModificationException` should be thrown.
* Maintain test cases
@artembilan artembilan added this to the 6.4.0-M1 milestone Jul 1, 2024
@artembilan artembilan added in: jdbc in: redis and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Jul 1, 2024
EddieChoCho added a commit to EddieChoCho/spring-integration that referenced this issue Jul 2, 2024
…y successful unlocking

Fixes: spring-projects#9291
* Modify `unlock()` method of `JdbcLock`: if the lock ownership can not be removed due to data expiration, a `ConcurrentModificationException` should be thrown.
* Modify `unlock()` method of `RedisLock`: if the lock ownership can not be removed due to data expiration, a `ConcurrentModificationException` should be thrown.
* Maintain test cases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants