refactor: remove parking_lot dependency and refine feature selections
- Trim
Cargo.tomlfeature selections - Remove
parking_lotdependency across the entire workspace in favor of standard library synchronization primitives
Trimming feature selections
I went through the dependency tree to apply default-features = false and trimming feature selections where possible.
Removing parking_lot
Some years ago I moved from std::sync::Mutex and friends to parking_lot when I was purging all use of unwrap and expect from the code base. Since then two things happened:
- Performance of
stdsynchronization primitives got a lot better; - I got wiser and now understand that when mutexes get poisoned, it's a good thing to propagate that error and panic the calling thread too. This is why we now use
expectagain here.
Fixes #1382
I didn't look fully through it, yet. But what is the behavior when the Mutex gets poisoned, did you test/encounter that, yet?
I didn’t test it. But you could trigger it by panicking while holding a lock. Which shouldn’t happen.
Edit: what will happen is that the thread trying to acquire the lock will then also panic. Purpose being to indeed have librespot exit rather than continue in a poisoned state.
So just to be sure if I understand it correct. If we encounter a poisoned lock, we don't want to continue as it can cause undefined behavior. This will probably not happen unless some access occurs to the lock while the thread of the active lock panics, in which case we already have a panic and just want to stop the program as a result. So the panic of the lock would just be a result of another error/panic, right?
Exactly.
So whats the state of things here @roderickvd? Could you resolve the conflicts and the review comment if everything is ready to merge?
Yeah if you guys are OK with it then I can do that.
I'm not sure if this fully fixes #1382, but besides that this sounds seems like a good thing to do. @kingosticks anything from your side that speaks against these changes?
Looks good to me.