Use std RwLock instead of parking_lot. #38
Conversation
|
Nice, many thanks! Kindly add some bried info to the commit log (and squash the additional commits into it while you're at it) on the rationale for the change in the details section? |
src/lib.rs
Outdated
|
|
||
| inner.inactive_receiver_count -= 1; | ||
| inner.close_channel(); | ||
| } | ||
| } | ||
|
|
||
| fn read_no_poison<T>(lock: &RwLock<T>) -> RwLockReadGuard<'_, T> { |
There was a problem hiding this comment.
Good work on keeping the existing behavior. But I'm not sure we should. 🤔 Since our locks are strictly internal only, the only code that can panic while holding the lock is our own and can leave us with some inconsistent internal state. Panicking on unlocking is likely the right thing to do here.
There was a problem hiding this comment.
Thanks for redoing this but I wasn't expecting your to be convinced by my arguments so quickly. 😆
We don't need these methods if we just panic. Just use inner.read().unwrap() and inner.write().unwrap().
There was a problem hiding this comment.
Also since we're changing the behavior about poisoning, it would be good to add a sentence about this in the commit log/PR description.
Sorry for being so pedantic. I'm sure this is the last time you've to re-work before we can merge.
There was a problem hiding this comment.
t I wasn't expecting your to be convinced by my arguments so q
Okey, then it make sense to remove read/write_no_poison at all =)
There was a problem hiding this comment.
t I wasn't expecting your to be convinced by my arguments so q
Okey, then it make sense to remove
read/write_no_poisonat all =)
Just to be clear, you mean you will remove these methods as per last suggestion or that you think we should keep the existing behavior of non poisoning? I'll need some arguments for both. :)
The reason I asked you to remove these is because 1. they're just duplicating unwrap 2. their names don't make any sense now.
There was a problem hiding this comment.
We don't need these methods
Ow, sorry, right, missed begin of your sentence.
Done. |
After sync primitives (Mutex, RwLock, Condvar) from rust std library start use futex in */Linux systems, they become same or faster than parking_lot sync primitives. That the reason switch to std sync primitives. Panic on poison instead of parking_lot behaviour.
Fixes #37.
After sync primitives (Mutex, RwLock, Condvar) from rust std library start use futex in */Linux systems, they become same or faster than parking_lot sync primitives. That the reason switch to std sync primitives.