Skip to content

feat: Implement write lock downgrading - #65

Merged
tisonkun merged 2 commits into
apache:mainfrom
orthur2:feat/add_downgrade_to_write_guard_in_rwlock
Aug 30, 2025
Merged

tisonkun merged 2 commits into
apache:mainfrom
orthur2:feat/add_downgrade_to_write_guard_in_rwlock

Conversation

@orthur2

@orthur2 orthur2 commented Aug 10, 2025 •

Copy link
Copy Markdown
Contributor

Description

This PR introduces a downgrade() method to RwLockWriteGuard and its variants, addressing Issue #62.

Currently, a task holding a write lock must drop it and re-acquire a read lock to allow concurrent reads. This is inefficient and creates a small window for race conditions. The downgrade() method provides a way to atomically and efficiently convert an exclusive write lock into a shared read lock, improving performance and ensuring safety.

Implementation

The downgrade() method has been added to all four RwLockWriteGuard variants:

  • RwLockWriteGuard
  • MappedRwLockWriteGuard
  • OwnedRwLockWriteGuard
  • OwnedMappedRwLockWriteGuard

The implementation atomically converts the lock by releasing max_readers - 1 permits from the underlying semaphore, leaving one permit for the new read guard. ManuallyDrop is used to prevent the original write guard's Drop implementation from running, ensuring panic safety and correct ownership transfer of the lock.

To implement RwLockWriteGuard::downgrade, I'll need to create a new RwLockReadGuard. However, the lock field of RwLockReadGuard is private. To avoid changing the access permissions of the lock field directly (given that changing it to pub(super) would grant excessive access, and Rust doesn't offer more fine-grained access control, which is a trade-off point tison and I discussed in PR #61), So, my current plan is to add a dedicated constructor for RwLockReadGuard. I will name it from_write_downgrade() instead of generic new() and constrain its API by requiring the WriteGuard to be passed in. This makes its purpose clear and prevents misuse.

@orthur2
orthur2 marked this pull request as draft August 10, 2025 17:27
@orthur2
orthur2 force-pushed the feat/add_downgrade_to_write_guard_in_rwlock branch from c18218b to b3ab854 Compare August 21, 2025 17:01
@tisonkun

Copy link
Copy Markdown
Member

Once PR #64 is merged, the tests are expected to pass.

#64 has been merged. I suppose you can rebase and make this PR ready for review then :D

@orthur2
orthur2 force-pushed the feat/add_downgrade_to_write_guard_in_rwlock branch from 3355ceb to 947cb1d Compare August 22, 2025 16:20
@orthur2
orthur2 marked this pull request as ready for review August 22, 2025 16:22
@orthur2

orthur2 commented Aug 22, 2025 •

Copy link
Copy Markdown
Contributor Author

Once PR #64 is merged, the tests are expected to pass.

#64 has been merged. I suppose you can rebase and make this PR ready for review then :D

Ready. @tisonkun

Signed-off-by: tison <wander4096@gmail.com>

@tisonkun tisonkun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. I made some code style changes and this is ready for merging.

@tisonkun
tisonkun enabled auto-merge (squash) August 30, 2025 15:26
@tisonkun
tisonkun merged commit 5cab666 into apache:main Aug 30, 2025
14 of 16 checks passed
@orthur2

orthur2 commented Aug 30, 2025

Copy link
Copy Markdown
Contributor Author

LGTM. I made some code style changes and this is ready for merging.

Thanks for your help.

@orthur2
orthur2 deleted the feat/add_downgrade_to_write_guard_in_rwlock branch August 30, 2025 16:52
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