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

[Feature Request] panic when user try get recursive lock. #438

Open
cathaysia opened this issue Apr 19, 2024 · 5 comments
Open

[Feature Request] panic when user try get recursive lock. #438

cathaysia opened this issue Apr 19, 2024 · 5 comments

Comments

@cathaysia
Copy link

I had notice that rwlock is not a recursive lock:

This lock uses a task-fair locking policy which avoids both reader and writer starvation. This means that readers trying to acquire the lock will block even if the lock is unlocked when there are writers waiting to acquire the lock. Because of this, attempts to recursively acquire a read lock within a single thread may result in a deadlock.

That is to say, I cannot acquire the read lock and write lock at the same time in one thread.

On Linux, trying to acquire a recursive lock on a non-recursive lock causes the program to crash. Should packing_lot also have this behavior? Crashing the program early can reduce debugging time (especially for problems like deadlocks)

@timnewsham
Copy link

I came here to make the same feature request:
There should be an optional feature to panic on duplicate read locks from the same thread.
This would be useful when running test cases. As it stands now, if you wrote a test case where a thread grabbed the same read lock twice it will pass just fine, unless you manage to get a concurrent write lock to occur. This is not practical in testing.

@Amanieu
Copy link
Owner

Amanieu commented Sep 4, 2024

I'm not opposed to having this as an optional feature, but you need to be aware that this will severely hurt performance. Essentially this will require a thread-local HashSet of all read locks that the current thread already owns. And it won't catch lock cycles across multiple threads.

@cathaysia
Copy link
Author

Yes, it will hurt performance. But it's very useful for debug. ;)

@Amanieu
Copy link
Owner

Amanieu commented Sep 4, 2024

I'm happy to accept a PR if you are willing to write one.

@cathaysia
Copy link
Author

I am glad to hear this. I will try to take time to do this.

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

No branches or pull requests

3 participants