Implement Arc::try_unwrap #262
Conversation
src/sync/arc.rs
Outdated
| let arc_value = unsafe { | ||
| let _arc_obj = ptr::read(&this.obj); | ||
| let arc_value = ptr::read(&this.value); | ||
|
|
||
| mem::forget(this); | ||
|
|
||
| arc_value | ||
| }; |
There was a problem hiding this comment.
a potential safe alternative to this is to wrap the loom Arc's value field in an Option so that we can take it out without dropping the struct...but, this would mean every other time the Arc's value is accessed would have to unwrap() it, which might have performance implications and make tests slower...this might be worth the unsafe code to avoid that?
it would be nice to add a comment explaining why this is safe, though.
There was a problem hiding this comment.
I don't have a strong preference. Current approach is borrowed from libstd. Added a comment now.
There was a problem hiding this comment.
yeah, i think doing it similarly to the way it's implemented in std is fine. thanks for the comment!
|
please take another look! |
| pub fn try_unwrap(this: Arc<T>) -> Result<T, Arc<T>> { | ||
| if !this.obj.get_mut(location!()) { | ||
| return Err(this); | ||
| } |
There was a problem hiding this comment.
We should probably have a test that the error case works.
tests/arc.rs
Outdated
| @@ -77,3 +77,13 @@ fn detect_mem_leak() { | |||
| std::mem::forget(num); | |||
| }); | |||
| } | |||
|
|
|||
| #[test] | |||
| fn try_unwrap() { | |||
There was a problem hiding this comment.
let's also add a test that try_unwrap fails when there's more than one ref?
# 0.5.6 (May 19, 2022) ### Added - cell: add `UnsafeCell::into_inner` for parity with `std` (#272) - sync: re-enable `Arc::strong_count` (#172) - sync: implement `Arc::try_unwrap` (#262) - sync: add `mpsc::Receiver::try_recv` (#262) ### Documented - show feature flags in docs (#151) - fix broken RustDoc links (#273)
# 0.5.6 (May 19, 2022) ### Added - cell: add `UnsafeCell::into_inner` for parity with `std` (#272) - sync: re-enable `Arc::strong_count` (#172) - sync: implement `Arc::try_unwrap` (#262) - sync: add `mpsc::Receiver::try_recv` (#262) ### Documented - show feature flags in docs (#151) - fix broken RustDoc links (#273)
|
Thank you for adding tests and merging! |
Fixes #261
I'm not sure if the unsafe destruction path is the best here.