From 57fd6cf32b2abc119f346065cbd2720034a6a2fa Mon Sep 17 00:00:00 2001 From: ThePuzzlemaker Date: Wed, 6 Jan 2021 17:18:12 -0600 Subject: [PATCH] Arc revisions (pt1/2(+?)) --- src/SUMMARY.md | 1 - src/arc-base.md | 76 +++++++++++++++++++++++++++++++++-------------- src/arc-clone.md | 54 ++++++++++++++++++++++++++------- src/arc-deref.md | 25 ---------------- src/arc-drop.md | 29 +++++++++++------- src/arc-final.md | 43 ++++++++++++++------------- src/arc-layout.md | 29 ++++++++++++------ 7 files changed, 158 insertions(+), 99 deletions(-) delete mode 100644 src/arc-deref.md diff --git a/src/SUMMARY.md b/src/SUMMARY.md index c732726..fbd9cc0 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -60,7 +60,6 @@ * [Base Code](arc-base.md) * [Cloning](arc-clone.md) * [Dropping](arc-drop.md) - * [Deref](arc-deref.md) * [Final Code](arc-final.md) * [FFI](ffi.md) * [Beneath `std`](beneath-std.md) diff --git a/src/arc-base.md b/src/arc-base.md index c561e19..7dd6bae 100644 --- a/src/arc-base.md +++ b/src/arc-base.md @@ -10,11 +10,6 @@ We'll first need a way to construct an `Arc`. This is pretty simple, as we just need to box the `ArcInner` and get a `NonNull` pointer to it. -We start the reference counter at 1, as that first reference is the current -pointer. As the `Arc` is cloned or dropped, it is updated. It is okay to call -`unwrap()` on the `Option` returned by `NonNull` as `Box::into_raw` guarantees -that the pointer returned is not null. - ```rust,ignore impl Arc { pub fn new(data: T) -> Arc { @@ -28,7 +23,7 @@ impl Arc { // It is okay to call `.unwrap()` here as we get a pointer from // `Box::into_raw` which is guaranteed to not be null. ptr: NonNull::new(Box::into_raw(boxed)).unwrap(), - _marker: PhantomData, + phantom: PhantomData, } } } @@ -43,8 +38,8 @@ more information on these, see [the section on `Send` and This is okay because: * You can only get a mutable reference to the value inside an `Arc` if and only - if it is the only `Arc` referencing that data -* We use atomic counters for reference counting + if it is the only `Arc` referencing that data (which only happens in `Drop`) +* We use atomics for the shared mutable reference counting ```rust,ignore unsafe impl Send for Arc {} @@ -56,26 +51,59 @@ bounds, it would be possible to share values that are thread-unsafe across a thread boundary via an `Arc`, which could possibly cause data races or unsoundness. -## Getting the `ArcInner` +For example, if those bounds were not present, `Arc>` would be `Sync` or +`Send`, meaning that you could clone the `Rc` out of the `Arc` to send it across +a thread (without creating an entirely new `Rc`), which would create data races +as `Rc` is not thread-safe. -We'll now want to make a private helper function, `inner()`, which just returns -the dereferenced `NonNull` pointer. +## Getting the `ArcInner` To dereference the `NonNull` pointer into a `&T`, we can call `NonNull::as_ref`. This is unsafe, unlike the typical `as_ref` function, so we must call it like this: ```rust,ignore -// inside the impl Arc block from before: -fn inner(&self) -> &ArcInner { - unsafe { self.ptr.as_ref() } -} +unsafe { self.ptr.as_ref() } ``` +We'll be using this snippet a few times in this code (usually with an associated +`let` binding). + This unsafety is okay because while this `Arc` is alive, we're guaranteed that the inner pointer is valid. +## Deref + +Alright. Now we can make `Arc`s (and soon will be able to clone and destroy them correctly), but how do we get +to the data inside? + +What we need now is an implementation of `Deref`. + +We'll need to import the trait: +```rust,ignore +use std::ops::Deref; +``` + +And here's the implementation: +```rust,ignore +impl Deref for Arc { + type Target = T; + + fn deref(&self) -> &T { + let inner = unsafe { self.ptr.as_ref() }; + &inner.data + } +} +``` + +Pretty simple, eh? This simply dereferences the `NonNull` pointer to the +`ArcInner`, then gets a reference to the data inside. + +## Code + Here's all the code from this section: ```rust,ignore +use std::ops::Deref; + impl Arc { pub fn new(data: T) -> Arc { // We start the reference count at 1, as that first reference is the @@ -88,17 +116,21 @@ impl Arc { // It is okay to call `.unwrap()` here as we get a pointer from // `Box::into_raw` which is guaranteed to not be null. ptr: NonNull::new(Box::into_raw(boxed)).unwrap(), - _marker: PhantomData, + phantom: PhantomData, } } - - fn inner(&self) -> &ArcInner { - // This unsafety is okay because while this Arc is alive, we're - // guaranteed that the inner pointer is valid. - unsafe { self.ptr.as_ref() } - } } unsafe impl Send for Arc {} unsafe impl Sync for Arc {} + + +impl Deref for Arc { + type Target = T; + + fn deref(&self) -> &T { + let inner = unsafe { self.ptr.as_ref() }; + &inner.data + } +} ``` diff --git a/src/arc-clone.md b/src/arc-clone.md index 42325f0..cfa1db6 100644 --- a/src/arc-clone.md +++ b/src/arc-clone.md @@ -3,13 +3,17 @@ Now that we've got some basic code set up, we'll need a way to clone the `Arc`. Basically, we need to: -1. Get the `ArcInner` value of the `Arc` -2. Increment the atomic reference count -3. Construct a new instance of the `Arc` from the inner pointer +1. Increment the atomic reference count +2. Construct a new instance of the `Arc` from the inner pointer -Next, we can update the atomic reference count as follows: +First, we need to get access to the `ArcInner`: ```rust,ignore -self.inner().rc.fetch_add(1, Ordering::Relaxed); +let inner = unsafe { self.ptr.as_ref() }; +``` + +We can update the atomic reference count as follows: +```rust,ignore +let old_rc = inner.rc.fetch_add(1, Ordering::Relaxed); ``` As described in [the standard library's implementation of `Arc` cloning][2]: @@ -30,15 +34,37 @@ We'll need to add another import to use `Ordering`: use std::sync::atomic::Ordering; ``` -It is possible that in some contrived programs (e.g. using `mem::forget`) that -the reference count could overflow, but it's unreasonable that would happen in -any reasonable program. +However, we have one problem with this implementation right now. What if someone +decides to `mem::forget` a bunch of Arcs? The code we have written so far (and +will write) assumes that the reference count accurately portrays how many Arcs +are in memory, but with `mem::forget` this is false. Thus, when more and more +Arcs are cloned from this one without them being `Drop`ped and the reference +count being decremented, we can overflow! This will cause use-after-free which +is **INCREDIBLY BAD!** + +To handle this, we need to check that the reference count does not go over some +arbitrary value (below `usize::MAX`, as we're storing the reference count as an +`AtomicUsize`), and do *something*. + +The standard library's implementation decides to just abort the program (as it +is an incredibly unlikely case in normal code and if it happens, the program is +probably incredibly degenerate) if the reference count reaches `isize::MAX` +(about half of `usize::MAX`) on any thread, on the assumption that there are +probably not about 2 billion threads (or about **9 quintillion** on some 64-bit +machines) incrementing the reference count at once. This is what we'll do. + +It's pretty simple to implement this behaviour: +```rust,ignore +if old_rc >= isize::MAX { + std::process::abort(); +} +``` Then, we need to return a new instance of the `Arc`: ```rust,ignore Self { ptr: self.ptr, - _marker: PhantomData + phantom: PhantomData } ``` @@ -48,12 +74,18 @@ use std::sync::atomic::Ordering; impl Clone for Arc { fn clone(&self) -> Arc { + let inner = unsafe { self.ptr.as_ref() }; // Using a relaxed ordering is alright here as knowledge of the original // reference prevents other threads from wrongly deleting the object. - self.inner().rc.fetch_add(1, Ordering::Relaxed); + inner.rc.fetch_add(1, Ordering::Relaxed); + + if old_rc >= isize::MAX { + std::process::abort(); + } + Self { ptr: self.ptr, - _marker: PhantomData, + phantom: PhantomData, } } } diff --git a/src/arc-deref.md b/src/arc-deref.md deleted file mode 100644 index 6dd04fe..0000000 --- a/src/arc-deref.md +++ /dev/null @@ -1,25 +0,0 @@ -# Deref - -Alright. We now have a way to make, clone, and destroy `Arc`s, but how do we get -to the data inside? - -What we need now is an implementation of `Deref`. - -We'll need to import the trait: -```rust,ignore -use std::ops::Deref; -``` - -And here's the implementation: -```rust,ignore -impl Deref for Arc { - type Target = T; - - fn deref(&self) -> &T { - &self.inner().data - } -} -``` - -Pretty simple, eh? This simply dereferences the `NonNull` pointer to the -`ArcInner`, then gets a reference to the data inside. diff --git a/src/arc-drop.md b/src/arc-drop.md index 893a15a..be65754 100644 --- a/src/arc-drop.md +++ b/src/arc-drop.md @@ -6,22 +6,28 @@ low enough, otherwise the data will live forever on the heap. To do this, we can implement `Drop`. Basically, we need to: -1. Get the `ArcInner` value of the `Arc` -2. Decrement the reference count -3. If there is only one reference remaining to the data, then: -4. Atomically fence the data to prevent reordering of the use and deletion of - the data, then: -5. Drop the inner data +1. Decrement the reference count +2. If there is only one reference remaining to the data, then: +3. Atomically fence the data to prevent reordering of the use and deletion of + the data +4. Drop the inner data -Now, we need to decrement the reference count. We can also bring in step 3 by -returning if the reference count is not equal to 1 (as `fetch_sub` returns the -previous value): +First, we'll need to get access to the `ArcInner`: ```rust,ignore -if self.inner().rc.fetch_sub(1, Ordering::Release) != 1 { +let inner = unsafe { self.ptr.as_ref() }; +``` + +Now, we need to decrement the reference count. To streamline our code, we can +also return if the returned value from `fetch_sub` (the value of the reference +count before decrementing it) is not equal to `1` (which happens when we are not +the last reference to the data). +```rust,ignore +if inner.rc.fetch_sub(1, Ordering::???) != 1 { return; } ``` +TODO We then need to create an atomic fence to prevent reordering of the use of the data and deletion of the data. As described in [the standard library's implementation of `Arc`][3]: @@ -78,7 +84,8 @@ Now, let's wrap this all up inside the `Drop` implementation: ```rust,ignore impl Drop for Arc { fn drop(&mut self) { - if self.inner().rc.fetch_sub(1, Ordering::Release) != 1 { + let inner = unsafe { self.ptr.as_ref() }; + if inner.rc.fetch_sub(1, Ordering::Release) != 1 { return; } // This fence is needed to prevent reordering of the use and deletion diff --git a/src/arc-final.md b/src/arc-final.md index 41d364c..1bcec56 100644 --- a/src/arc-final.md +++ b/src/arc-final.md @@ -9,7 +9,7 @@ use std::sync::atomic::{self, AtomicUsize, Ordering}; pub struct Arc { ptr: NonNull>, - _marker: PhantomData>, + phantom: PhantomData>, } pub struct ArcInner { @@ -29,36 +29,47 @@ impl Arc { // It is okay to call `.unwrap()` here as we get a pointer from // `Box::into_raw` which is guaranteed to not be null. ptr: NonNull::new(Box::into_raw(boxed)).unwrap(), - _marker: PhantomData, + phantom: PhantomData, } } - - fn inner(&self) -> &ArcInner { - // This unsafety is okay because while this Arc is alive, we're - // guaranteed that the inner pointer is valid. Also, ArcInner is - // Sync if T is Sync. - unsafe { self.ptr.as_ref() } - } } unsafe impl Send for Arc {} unsafe impl Sync for Arc {} +impl Deref for Arc { + type Target = T; + + fn deref(&self) -> &T { + let inner = unsafe { self.ptr.as_ref() }; + &inner.data + } +} + +use std::sync::atomic::Ordering; + impl Clone for Arc { fn clone(&self) -> Arc { + let inner = unsafe { self.ptr.as_ref() }; // Using a relaxed ordering is alright here as knowledge of the original // reference prevents other threads from wrongly deleting the object. - self.inner().rc.fetch_add(1, Ordering::Relaxed); + inner.rc.fetch_add(1, Ordering::Relaxed); + + if old_rc >= isize::MAX { + std::process::abort(); + } + Self { ptr: self.ptr, - _marker: PhantomData, + phantom: PhantomData, } } } impl Drop for Arc { fn drop(&mut self) { - if self.inner().rc.fetch_sub(1, Ordering::Release) != 1 { + let inner = unsafe { self.ptr.as_ref() }; + if inner.rc.fetch_sub(1, Ordering::Release) != 1 { return; } // This fence is needed to prevent reordering of the use and deletion @@ -69,12 +80,4 @@ impl Drop for Arc { unsafe { Box::from_raw(self.ptr.as_ptr()); } } } - -impl Deref for Arc { - type Target = T; - - fn deref(&self) -> &T { - &self.inner().data - } -} ``` diff --git a/src/arc-layout.md b/src/arc-layout.md index a8a4c86..55d4793 100644 --- a/src/arc-layout.md +++ b/src/arc-layout.md @@ -2,14 +2,25 @@ Let's start by making the layout for our implementation of `Arc`. -We'll need at least two things: a pointer to our data and a shared atomic -reference count. Since we're *building* `Arc`, we can't store the reference -count inside another `Arc`. Instead, we can get the best of both worlds and -store our data and the reference count in one structure and get a pointer to -that. This also prevents having to dereference two separate pointers inside our -code, which may also improve performance. (Yay!) - -Naively, it'd looks something like this: +An `Arc` provides thread-safe shared ownership of a value of type `T`, +allocated in the heap. Sharing implies immutability in Rust, so we don't need to +design anything that manages access to that value, right? Although interior +mutability types like Mutex allow Arc's users to create shared mutability, Arc +itself doesn't need to concern itself with these issues. + +However there _is_ one place where Arc needs to concern itself with mutation: +destruction. When all the owners of the Arc go away, we need to be able to +`drop` its contents and free its allocation. So we need a way for an owner to +know if it's the _last_ owner, and the simplest way to do that is with a count +of the owners -- Reference Counting. + +Unfortunately, this reference count is inherently shared mutable state, so Arc +_does_ need to think about synchronization. We _could_ use a Mutex for this, but +that's overkill. Instead, we'll use atomics. And since everyone already needs a +pointer to the T's allocation, we might as well put the reference count in that +same allocation. + +Naively, it would look something like this: ```rust,ignore use std::sync::atomic; @@ -41,7 +52,7 @@ To fix the second problem, we can include a `PhantomData` marker containing an `ArcInner`. This will tell the drop checker that we have some notion of ownership of a value of `ArcInner` (which itself contains some `T`). -With these changes, our new structure will look like this: +With these changes we get our final structure: ```rust,ignore use std::marker::PhantomData; use std::ptr::NonNull;