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

refactor(bdev/lock): add range lock trait #51

Closed
wants to merge 1 commit into from

Conversation

tiagolobocastro
Copy link
Contributor

@tiagolobocastro tiagolobocastro commented Jan 12, 2024

Makes the range locking an interface which can be implemented by different types.


/// Allows gaining exclusive access over a block range.
/// # Warning
/// Due to the lack of async drop the range lock must be
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if we can still release in the drop method, just not wait for it - and then the caller may decide to either wait or just allow the drop to do it if it doesn't care about waiting for unlock to complete, wdyt @dsavitskiy ?

Copy link
Contributor

Choose a reason for hiding this comment

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

As I understand this drop part - since we have nothing custom to do in Drop, and the allocated heap memory will be anyway freed by default drop, it seems ok to wait for unlock to complete.

@dsharma-dc
Copy link
Contributor

What's the rationale for this change? Perhaps you can update in PR description.

@tiagolobocastro
Copy link
Contributor Author

What's the rationale for this change? Perhaps you can update in PR description.

done


/// Allows gaining exclusive access over a block range.
/// # Warning
/// Due to the lack of async drop the range lock must be
Copy link
Contributor

Choose a reason for hiding this comment

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

As I understand this drop part - since we have nothing custom to do in Drop, and the allocated heap memory will be anyway freed by default drop, it seems ok to wait for unlock to complete.

@tiagolobocastro
Copy link
Contributor Author

I ended up not requiring this, so closing it for now.

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.

4 participants