Skip to content

Commit 28081a6

Browse files
authored
Rollup merge of rust-lang#106854 - steffahn:drop_linear_arc_rebased, r=Mark-Simulacrum
Add `Arc::into_inner` for safely discarding `Arc`s without calling the destructor on the inner type. ACP: rust-lang/libs-team#162 Reviving rust-lang#79665. I want to get this merged this time; this does not contain changes (apart from very minor changes in comments/docs). See rust-lang#79665 for further description of the PR. The only “unresolved” points that led to that PR being closed, AFAICT, were * The desire to also implement a `Rc::into_inner` function * however, this can very well also happen as a subsequent PR * Possible need for further discussion on the naming “`into_inner`” (?) * `into_inner` seems fine to me; also, this PR introduces unstable API, and names can be changed later, too * ~~I don't know if a tracking issue for the feature flag is supposed to be opened before or after this PR gets merged (if *before*, then I can add the issue number to the `#[unstable…]` attribute)~~ There is a [tracking issue](rust-lang#106894) now. I say “unresolved” in quotation marks because from my point of view, if reviewers agree, the PR can be merged immediately and as-is :-)
2 parents 3d4c312 + 33696fa commit 28081a6

File tree

2 files changed

+177
-0
lines changed

2 files changed

+177
-0
lines changed

library/alloc/src/sync.rs

+145
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,20 @@ impl<T> Arc<T> {
654654
///
655655
/// This will succeed even if there are outstanding weak references.
656656
///
657+
// FIXME: when `Arc::into_inner` is stabilized, add this paragraph:
658+
/*
659+
/// It is strongly recommended to use [`Arc::into_inner`] instead if you don't
660+
/// want to keep the `Arc` in the [`Err`] case.
661+
/// Immediately dropping the [`Err`] payload, like in the expression
662+
/// `Arc::try_unwrap(this).ok()`, can still cause the strong count to
663+
/// drop to zero and the inner value of the `Arc` to be dropped:
664+
/// For instance if two threads execute this expression in parallel, then
665+
/// there is a race condition. The threads could first both check whether they
666+
/// have the last clone of their `Arc` via `Arc::try_unwrap`, and then
667+
/// both drop their `Arc` in the call to [`ok`][`Result::ok`],
668+
/// taking the strong count from two down to zero.
669+
///
670+
*/
657671
/// # Examples
658672
///
659673
/// ```
@@ -685,6 +699,137 @@ impl<T> Arc<T> {
685699
Ok(elem)
686700
}
687701
}
702+
703+
/// Returns the inner value, if the `Arc` has exactly one strong reference.
704+
///
705+
/// Otherwise, [`None`] is returned and the `Arc` is dropped.
706+
///
707+
/// This will succeed even if there are outstanding weak references.
708+
///
709+
/// If `Arc::into_inner` is called on every clone of this `Arc`,
710+
/// it is guaranteed that exactly one of the calls returns the inner value.
711+
/// This means in particular that the inner value is not dropped.
712+
///
713+
/// The similar expression `Arc::try_unwrap(this).ok()` does not
714+
/// offer such a guarantee. See the last example below.
715+
//
716+
// FIXME: when `Arc::into_inner` is stabilized, add this to end
717+
// of the previous sentence:
718+
/*
719+
/// and the documentation of [`Arc::try_unwrap`].
720+
*/
721+
///
722+
/// # Examples
723+
///
724+
/// Minimal example demonstrating the guarantee that `Arc::into_inner` gives.
725+
/// ```
726+
/// #![feature(arc_into_inner)]
727+
///
728+
/// use std::sync::Arc;
729+
///
730+
/// let x = Arc::new(3);
731+
/// let y = Arc::clone(&x);
732+
///
733+
/// // Two threads calling `Arc::into_inner` on both clones of an `Arc`:
734+
/// let x_thread = std::thread::spawn(|| Arc::into_inner(x));
735+
/// let y_thread = std::thread::spawn(|| Arc::into_inner(y));
736+
///
737+
/// let x_inner_value = x_thread.join().unwrap();
738+
/// let y_inner_value = y_thread.join().unwrap();
739+
///
740+
/// // One of the threads is guaranteed to receive the inner value:
741+
/// assert!(matches!(
742+
/// (x_inner_value, y_inner_value),
743+
/// (None, Some(3)) | (Some(3), None)
744+
/// ));
745+
/// // The result could also be `(None, None)` if the threads called
746+
/// // `Arc::try_unwrap(x).ok()` and `Arc::try_unwrap(y).ok()` instead.
747+
/// ```
748+
///
749+
/// A more practical example demonstrating the need for `Arc::into_inner`:
750+
/// ```
751+
/// #![feature(arc_into_inner)]
752+
///
753+
/// use std::sync::Arc;
754+
///
755+
/// // Definition of a simple singly linked list using `Arc`:
756+
/// #[derive(Clone)]
757+
/// struct LinkedList<T>(Option<Arc<Node<T>>>);
758+
/// struct Node<T>(T, Option<Arc<Node<T>>>);
759+
///
760+
/// // Dropping a long `LinkedList<T>` relying on the destructor of `Arc`
761+
/// // can cause a stack overflow. To prevent this, we can provide a
762+
/// // manual `Drop` implementation that does the destruction in a loop:
763+
/// impl<T> Drop for LinkedList<T> {
764+
/// fn drop(&mut self) {
765+
/// let mut link = self.0.take();
766+
/// while let Some(arc_node) = link.take() {
767+
/// if let Some(Node(_value, next)) = Arc::into_inner(arc_node) {
768+
/// link = next;
769+
/// }
770+
/// }
771+
/// }
772+
/// }
773+
///
774+
/// // Implementation of `new` and `push` omitted
775+
/// impl<T> LinkedList<T> {
776+
/// /* ... */
777+
/// # fn new() -> Self {
778+
/// # LinkedList(None)
779+
/// # }
780+
/// # fn push(&mut self, x: T) {
781+
/// # self.0 = Some(Arc::new(Node(x, self.0.take())));
782+
/// # }
783+
/// }
784+
///
785+
/// // The following code could have still caused a stack overflow
786+
/// // despite the manual `Drop` impl if that `Drop` impl had used
787+
/// // `Arc::try_unwrap(arc).ok()` instead of `Arc::into_inner(arc)`.
788+
///
789+
/// // Create a long list and clone it
790+
/// let mut x = LinkedList::new();
791+
/// for i in 0..100000 {
792+
/// x.push(i); // Adds i to the front of x
793+
/// }
794+
/// let y = x.clone();
795+
///
796+
/// // Drop the clones in parallel
797+
/// let x_thread = std::thread::spawn(|| drop(x));
798+
/// let y_thread = std::thread::spawn(|| drop(y));
799+
/// x_thread.join().unwrap();
800+
/// y_thread.join().unwrap();
801+
/// ```
802+
803+
// FIXME: when `Arc::into_inner` is stabilized, adjust above documentation
804+
// and the documentation of `Arc::try_unwrap` according to the `FIXME`s. Also
805+
// open an issue on rust-lang/rust-clippy, asking for a lint against
806+
// `Arc::try_unwrap(...).ok()`.
807+
#[inline]
808+
#[unstable(feature = "arc_into_inner", issue = "106894")]
809+
pub fn into_inner(this: Self) -> Option<T> {
810+
// Make sure that the ordinary `Drop` implementation isn’t called as well
811+
let mut this = mem::ManuallyDrop::new(this);
812+
813+
// Following the implementation of `drop` and `drop_slow`
814+
if this.inner().strong.fetch_sub(1, Release) != 1 {
815+
return None;
816+
}
817+
818+
acquire!(this.inner().strong);
819+
820+
// SAFETY: This mirrors the line
821+
//
822+
// unsafe { ptr::drop_in_place(Self::get_mut_unchecked(self)) };
823+
//
824+
// in `drop_slow`. Instead of dropping the value behind the pointer,
825+
// it is read and eventually returned; `ptr::read` has the same
826+
// safety conditions as `ptr::drop_in_place`.
827+
let inner = unsafe { ptr::read(Self::get_mut_unchecked(&mut this)) };
828+
829+
drop(Weak { ptr: this.ptr });
830+
831+
Some(inner)
832+
}
688833
}
689834

690835
impl<T> Arc<[T]> {

library/alloc/src/sync/tests.rs

+32
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,38 @@ fn try_unwrap() {
101101
assert_eq!(Arc::try_unwrap(x), Ok(5));
102102
}
103103

104+
#[test]
105+
fn into_inner() {
106+
for _ in 0..100
107+
// ^ Increase chances of hitting potential race conditions
108+
{
109+
let x = Arc::new(3);
110+
let y = Arc::clone(&x);
111+
let r_thread = std::thread::spawn(|| Arc::into_inner(x));
112+
let s_thread = std::thread::spawn(|| Arc::into_inner(y));
113+
let r = r_thread.join().expect("r_thread panicked");
114+
let s = s_thread.join().expect("s_thread panicked");
115+
assert!(
116+
matches!((r, s), (None, Some(3)) | (Some(3), None)),
117+
"assertion failed: unexpected result `{:?}`\
118+
\n expected `(None, Some(3))` or `(Some(3), None)`",
119+
(r, s),
120+
);
121+
}
122+
123+
let x = Arc::new(3);
124+
assert_eq!(Arc::into_inner(x), Some(3));
125+
126+
let x = Arc::new(4);
127+
let y = Arc::clone(&x);
128+
assert_eq!(Arc::into_inner(x), None);
129+
assert_eq!(Arc::into_inner(y), Some(4));
130+
131+
let x = Arc::new(5);
132+
let _w = Arc::downgrade(&x);
133+
assert_eq!(Arc::into_inner(x), Some(5));
134+
}
135+
104136
#[test]
105137
fn into_from_raw() {
106138
let x = Arc::new(Box::new("hello"));

0 commit comments

Comments
 (0)