Fix unsoundness #6
Conversation
| self | ||
| .value | ||
| .try_insert(value) | ||
| .map_err(|_| ()) |
There was a problem hiding this comment.
Why mapping the error to a unit and not just expecting/unwrapping the original error? Just curious.
There was a problem hiding this comment.
The error in this case is the value that we passed to try_insert in the line above. expect as well as unwrap require the error to implement Debug. So we'd have to add a T: Debug bound.
|
@hjiayz can you take a look at the PR 🙏🏼, and thanks for the lib btw! |
| drop(wakers); | ||
| return Poll::Ready(unsafe { &*ptr }); | ||
|
|
||
| let mut fut = self.fut.lock(); |
There was a problem hiding this comment.
Just noticed, that a second self.value.get() is required here, since the value might have been set by whoever held the mutex before, and polling a future after it's ready can lead to panics, depending on the implementation.
| @@ -17,6 +17,10 @@ license = 'MIT OR Apache-2.0' | |||
| name = "benchmark" | |||
| harness = false | |||
|
|
|||
| [dependencies] | |||
| parking_lot = "0.12.1" | |||
There was a problem hiding this comment.
At this point in time, parking_lot is no longer recommended over the std Mutex, since std now uses futex on Linux, which performs better in most cases.
Closes #5