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

Remove the limitation of try_lock being a protected function. #125461

Closed
wants to merge 1 commit into from

Conversation

RadiantUwU
Copy link

This honestly looks like it was completely arbitrary to be limited to pub(crate) even though it didnt need to be like this.

@rustbot
Copy link
Collaborator

rustbot commented May 23, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jhpratt (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 23, 2024
@jhpratt
Copy link
Member

jhpratt commented May 23, 2024

In the tracking issue (#121440), fallible locking is listed as an unresolved question due to the apparent lack of a use case. I am closing this PR for that reason; feel free to start a discussion in that issue regarding your use case if you have one.

@jhpratt jhpratt closed this May 23, 2024
@RadiantUwU
Copy link
Author

RadiantUwU commented May 24, 2024

In the tracking issue (#121440), fallible locking is listed as an unresolved question due to the apparent lack of a use case. I am closing this PR for that reason; feel free to start a discussion in that issue regarding your use case if you have one.

I'm trying to implement a ReentrantRwLock and this heavily depends on it.
I had to code my own ReentrantLock (mutex, whatever) for this instead as the stdlib cannot provide what i request.
https://github.com/godotengine/godot/pull/91682/files (This is how it looks in c++, i am currently making it in Rust as well)

The rust implementation of this requires a ReentrantLock that can try_lock.

@RadiantUwU
Copy link
Author

I would be more than happy to add a ReentrantRwLock to the std lib directly and make a pull request for it.

@RadiantUwU
Copy link
Author

I also failed to mention how Mutex and RwLock both have try_lock, while the ReentrantLock does not.

@jhpratt
Copy link
Member

jhpratt commented May 24, 2024

As I said, bring it up in the tracking issue. The relevant people aren't seeing your comments here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants