From a29fff0d7d0d7164dde6d56a532991546109b7e4 Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Mon, 22 Mar 2021 22:50:17 +0000 Subject: [PATCH 01/21] Add example of analyzing soundness of Send/Sync Add an example of thinking through whether it is sound to implement Send + Sync for a custom type that wraps a raw pointer. I read the existing docs and was confused about whether I could implement Send + Sync for a type I wrote that wraps a c-style array. Kixiron, InfernoDeity, Talchas, and HeroicKatora on #black-magic helped me understand Send and Sync better. This example is based on the advice they gave me. I've made lots of changes, so any errors are probably mine. --- src/send-and-sync.md | 107 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index 87d0d300..c2d26dc6 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -74,7 +74,114 @@ of their pervasive use of raw pointers to manage allocations and complex ownersh Similarly, most iterators into these collections are Send and Sync because they largely behave like an `&` or `&mut` into the collection. +[`Box`][box-doc] is implemented as it's own special intrinsic type by the +compiler for [various reasons][box-is-special], but we can implement something +with similar-ish behaviour ourselves to see an example of when it is sound to +implement Send and Sync. Let's call it a `Carton`. + +We start by writing code to take a value allocated on the stack and transfer it +to the heap. + +```rust +use std::mem::size_of; +use std::ptr::NonNull; + +struct Carton(NonNull); + +impl Carton { + pub fn new(mut value: T) -> Self { + // Allocate enough memory on the heap to store one T + let ptr = unsafe { libc::calloc(1, size_of::()) as *mut T }; + + // NonNull is just a wrapper that enforces that the pointer isn't null. + // Malloc returns null if it can't allocate. + let mut ptr = NonNull::new(ptr).expect("We assume malloc doesn't fail"); + + // Move value from the stack to the location we allocated on the heap + unsafe { + // Safety: The pointer returned by calloc is alligned, initialized, + // and dereferenceable, and we have exclusive access to the pointer. + *ptr.as_mut() = value; + } + + Self(ptr) + } +} +``` + +This isn't very useful, because once our users give us a value they have no way +to access it. [`Box`][box-doc] implements [`Deref`][deref-doc] and +[`DerefMut`][deref-mut-doc] so that you can access the inner value. Let's do +that. + +```rust +use std::ops::{Deref, DerefMut}; + +impl Deref for Carton { + type Target = T; + + fn deref(&self) -> &Self::Target { + unsafe { + // Safety: The pointer is aligned, initialized, and dereferenceable + // by the logic in [`Self::new`]. We require writers to borrow the + // Carton, and the lifetime of the return value is elided to the + // lifetime of the input. This means the borrow checker will + // enforce that no one can mutate the contents of the Carton until + // the reference returned is dropped. + self.0.as_ref() + } + } +} + +impl DerefMut for Carton { + fn deref_mut(&mut self) -> &mut Self::Target { + unsafe { + // Safety: The pointer is aligned, initialized, and dereferenceable + // by the logic in [`Self::new]. We require writers to mutably + // borrow the Carton, and the lifetime of the return value is + // elided to the lifetime of the input. This means the borrow + // checker will enforce that no one else can access the contents + // of the Carton until the mutable reference returned is dropped. + self.0.as_mut() + } + } +} +``` + +Finally, lets think about whether our `Carton` is Send and Sync. Something can +safely be Send unless it shares mutable state with something else without +enforcing exclusive access to it. Each `Carton` has a unique pointer, so +we're good. + +```rust +// Safety: No one besides us has the raw pointer, so we can safely transfer the +// Carton to another thread. +unsafe impl Send for Carton {} +``` + +What about Sync? For `Carton` to be Sync we have to enforce that you can't +write to something stored in a `&Carton` while that same something could be read +or written to from another `&Carton`. Since you need an `&mut Carton` to +write to the pointer, and the borrow checker enforces that mutable +references must be exclusive, there are no soundness issues making `Carton` +sync either. + +```rust +// Safety: Our implementation of DerefMut requires writers to mutably borrow the +// Carton, so the borrow checker will only let us have references to the Carton +// on multiple threads if no one has a mutable reference to the Carton. +unsafe impl Sync for Carton {} +``` + TODO: better explain what can or can't be Send or Sync. Sufficient to appeal only to data races? [unsafe traits]: safe-unsafe-meaning.html + +[box-doc]: https://doc.rust-lang.org/std/boxed/struct.Box.html + +[box-is-special]: https://manishearth.github.io/blog/2017/01/10/rust-tidbits-box-is-special/ + +[deref-doc]: https://doc.rust-lang.org/core/ops/trait.Deref.html + +[deref-mut-doc]: https://doc.rust-lang.org/core/ops/trait.DerefMut.html From 8739b01e18d885930ebf909a953a638351ca1a2e Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Mon, 22 Mar 2021 23:05:47 +0000 Subject: [PATCH 02/21] Don't execute example code Making the code executable would require adding a lot of duplicated hidden lines, plus libc isn't available. --- src/send-and-sync.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index c2d26dc6..321d4435 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -82,7 +82,7 @@ implement Send and Sync. Let's call it a `Carton`. We start by writing code to take a value allocated on the stack and transfer it to the heap. -```rust +```rust,ignore use std::mem::size_of; use std::ptr::NonNull; @@ -114,7 +114,7 @@ to access it. [`Box`][box-doc] implements [`Deref`][deref-doc] and [`DerefMut`][deref-mut-doc] so that you can access the inner value. Let's do that. -```rust +```rust,ignore use std::ops::{Deref, DerefMut}; impl Deref for Carton { @@ -153,7 +153,7 @@ safely be Send unless it shares mutable state with something else without enforcing exclusive access to it. Each `Carton` has a unique pointer, so we're good. -```rust +```rust,ignore // Safety: No one besides us has the raw pointer, so we can safely transfer the // Carton to another thread. unsafe impl Send for Carton {} @@ -166,7 +166,7 @@ write to the pointer, and the borrow checker enforces that mutable references must be exclusive, there are no soundness issues making `Carton` sync either. -```rust +```rust,ignore // Safety: Our implementation of DerefMut requires writers to mutably borrow the // Carton, so the borrow checker will only let us have references to the Carton // on multiple threads if no one has a mutable reference to the Carton. From c3ba1789e760dc034d1d49ba76af0e005d19557b Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Tue, 23 Mar 2021 20:27:37 +0000 Subject: [PATCH 03/21] Fix soundness issues in new danielhenrymantilla pointed out the issues. --- src/send-and-sync.md | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index 321d4435..69845d6e 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -89,19 +89,31 @@ use std::ptr::NonNull; struct Carton(NonNull); impl Carton { - pub fn new(mut value: T) -> Self { - // Allocate enough memory on the heap to store one T - let ptr = unsafe { libc::calloc(1, size_of::()) as *mut T }; + pub fn new(value: T) -> Self { + // Allocate enough memory on the heap to store one T. + let memptr = &mut null_mut() as *mut *mut T; + unsafe { + let ret = libc::posix_memalign( + memptr as *mut *mut c_void, + align_of::(), + size_of::() + ); + assert_eq!(ret, 0, "Failed to allocate or invalid alignment"); + }; // NonNull is just a wrapper that enforces that the pointer isn't null. - // Malloc returns null if it can't allocate. - let mut ptr = NonNull::new(ptr).expect("We assume malloc doesn't fail"); - - // Move value from the stack to the location we allocated on the heap + let mut ptr = unsafe { + // Safety: memptr is dereferenceable because we created it from a + // reference and have exclusive access. + NonNull::new(*memptr) + .expect("Guaranteed non-null if posix_memalign returns 0") + }; + + // Move value from the stack to the location we allocated on the heap. unsafe { - // Safety: The pointer returned by calloc is alligned, initialized, - // and dereferenceable, and we have exclusive access to the pointer. - *ptr.as_mut() = value; + // Safety: If non-null, posix_memalign gives us a ptr that is valid + // for writes and properly aligned. + ptr.as_ptr().write(value); } Self(ptr) From 05a2df52a497dbb2d5e2099b74e66fbcbfeac5b9 Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Tue, 23 Mar 2021 20:37:35 +0000 Subject: [PATCH 04/21] Fix missing trait bounds --- src/send-and-sync.md | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index 69845d6e..87c8b6a5 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -167,8 +167,8 @@ we're good. ```rust,ignore // Safety: No one besides us has the raw pointer, so we can safely transfer the -// Carton to another thread. -unsafe impl Send for Carton {} +// Carton to another thread if T can be safely transferred. +unsafe impl Send for Carton where T: Send {} ``` What about Sync? For `Carton` to be Sync we have to enforce that you can't @@ -181,8 +181,20 @@ sync either. ```rust,ignore // Safety: Our implementation of DerefMut requires writers to mutably borrow the // Carton, so the borrow checker will only let us have references to the Carton -// on multiple threads if no one has a mutable reference to the Carton. -unsafe impl Sync for Carton {} +// on multiple threads if no one has a mutable reference to the Carton. This +// means we are Sync if T is Sync. +unsafe impl Sync for Carton where T: Sync {} +``` + +When we assert our type is Send and Sync we need to enforce that every +contained type is Send and Sync. When writing custom types that behave like +standard library types we can assert that we have the same requirements. +For example, the following code asserts that a Carton is Send if the same +sort of Box would be Send, which in this case is the same as saying T is Send. + +```rust +# struct Carton(std::ptr::NonNull); +unsafe impl Send for Carton where Box: Send {} ``` TODO: better explain what can or can't be Send or Sync. Sufficient to appeal From 29f71397137b3b2506fd47ff2ead90dc980880a1 Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Tue, 23 Mar 2021 20:41:54 +0000 Subject: [PATCH 05/21] Reduce untested code --- src/send-and-sync.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index 87c8b6a5..b4105bf2 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -126,9 +126,11 @@ to access it. [`Box`][box-doc] implements [`Deref`][deref-doc] and [`DerefMut`][deref-mut-doc] so that you can access the inner value. Let's do that. -```rust,ignore +```rust use std::ops::{Deref, DerefMut}; +# struct Carton(std::ptr::NonNull); + impl Deref for Carton { type Target = T; @@ -165,7 +167,8 @@ safely be Send unless it shares mutable state with something else without enforcing exclusive access to it. Each `Carton` has a unique pointer, so we're good. -```rust,ignore +```rust +# struct Carton(std::ptr::NonNull); // Safety: No one besides us has the raw pointer, so we can safely transfer the // Carton to another thread if T can be safely transferred. unsafe impl Send for Carton where T: Send {} @@ -178,7 +181,8 @@ write to the pointer, and the borrow checker enforces that mutable references must be exclusive, there are no soundness issues making `Carton` sync either. -```rust,ignore +```rust +# struct Carton(std::ptr::NonNull); // Safety: Our implementation of DerefMut requires writers to mutably borrow the // Carton, so the borrow checker will only let us have references to the Carton // on multiple threads if no one has a mutable reference to the Carton. This From 2dd3566f407b6d4958e6faaff4236539eba117f0 Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Tue, 23 Mar 2021 20:44:56 +0000 Subject: [PATCH 06/21] Use american spelling Co-authored-by: Yuki Okushi --- src/send-and-sync.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index b4105bf2..a5b3dac7 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -76,7 +76,7 @@ largely behave like an `&` or `&mut` into the collection. [`Box`][box-doc] is implemented as it's own special intrinsic type by the compiler for [various reasons][box-is-special], but we can implement something -with similar-ish behaviour ourselves to see an example of when it is sound to +with similar-ish behavior ourselves to see an example of when it is sound to implement Send and Sync. Let's call it a `Carton`. We start by writing code to take a value allocated on the stack and transfer it From 357eab4857944d5b51dc5655ef794395b7460d26 Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Tue, 23 Mar 2021 20:45:13 +0000 Subject: [PATCH 07/21] Fix formatting Co-authored-by: Yuki Okushi --- src/send-and-sync.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index a5b3dac7..3209a6a9 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -205,11 +205,7 @@ TODO: better explain what can or can't be Send or Sync. Sufficient to appeal only to data races? [unsafe traits]: safe-unsafe-meaning.html - [box-doc]: https://doc.rust-lang.org/std/boxed/struct.Box.html - [box-is-special]: https://manishearth.github.io/blog/2017/01/10/rust-tidbits-box-is-special/ - [deref-doc]: https://doc.rust-lang.org/core/ops/trait.Deref.html - [deref-mut-doc]: https://doc.rust-lang.org/core/ops/trait.DerefMut.html From e542c32c45b6b1591fdbc694e6c0d669eb58d3dc Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Tue, 23 Mar 2021 20:47:37 +0000 Subject: [PATCH 08/21] Add heading --- src/send-and-sync.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index 3209a6a9..fd1bb94d 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -74,6 +74,8 @@ of their pervasive use of raw pointers to manage allocations and complex ownersh Similarly, most iterators into these collections are Send and Sync because they largely behave like an `&` or `&mut` into the collection. +## Example + [`Box`][box-doc] is implemented as it's own special intrinsic type by the compiler for [various reasons][box-is-special], but we can implement something with similar-ish behavior ourselves to see an example of when it is sound to From 75253ec543b38cfab3bfb9d8b49bcb5971c0c7e1 Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Tue, 23 Mar 2021 21:14:34 +0000 Subject: [PATCH 09/21] Make out param stack local, fix for zero sized types Co-authored-by: Daniel Henry-Mantilla --- src/send-and-sync.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index fd1bb94d..33860ef5 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -93,10 +93,11 @@ struct Carton(NonNull); impl Carton { pub fn new(value: T) -> Self { // Allocate enough memory on the heap to store one T. - let memptr = &mut null_mut() as *mut *mut T; + assert_ne!(size_of::(), 0, "Zero-sized types are out of the scope of this example); + let memptr: *mut c_void = ptr::null_mut(); unsafe { let ret = libc::posix_memalign( - memptr as *mut *mut c_void, + &mut memptr, align_of::(), size_of::() ); @@ -107,7 +108,7 @@ impl Carton { let mut ptr = unsafe { // Safety: memptr is dereferenceable because we created it from a // reference and have exclusive access. - NonNull::new(*memptr) + NonNull::new(memptr.cast::()) .expect("Guaranteed non-null if posix_memalign returns 0") }; From 53661a5343d6802227b59d1ad9eee22617e63651 Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Tue, 23 Mar 2021 21:19:01 +0000 Subject: [PATCH 10/21] Description fixes by danielhenrymantilla Co-authored-by: Daniel Henry-Mantilla --- src/send-and-sync.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index 33860ef5..f017b9c4 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -186,14 +186,16 @@ sync either. ```rust # struct Carton(std::ptr::NonNull); -// Safety: Our implementation of DerefMut requires writers to mutably borrow the -// Carton, so the borrow checker will only let us have references to the Carton -// on multiple threads if no one has a mutable reference to the Carton. This -// means we are Sync if T is Sync. +// Safety: Since there exists a public way to go from a `&Carton` to a `&T` +// in an unsynchronized fashion (such as `Deref`), then `Carton` can't be +// `Sync` if `T` isn't. +// Conversely, `Carton` itself does not use any interior mutability whatsoever: +// all the mutations are performed through an exclusive reference (`&mut`). This +// means it suffices that `T` be `Sync` for `Carton` to be `Sync`: unsafe impl Sync for Carton where T: Sync {} ``` -When we assert our type is Send and Sync we need to enforce that every +When we assert our type is Send and Sync we usually need to enforce that every contained type is Send and Sync. When writing custom types that behave like standard library types we can assert that we have the same requirements. For example, the following code asserts that a Carton is Send if the same From 9f9f909d9149153008da3c7629ae497d9b05a545 Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Tue, 23 Mar 2021 21:19:54 +0000 Subject: [PATCH 11/21] Qualify with ptr:: --- src/send-and-sync.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index f017b9c4..ee161abe 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -86,9 +86,9 @@ to the heap. ```rust,ignore use std::mem::size_of; -use std::ptr::NonNull; +use std::ptr; -struct Carton(NonNull); +struct Carton(ptr::NonNull); impl Carton { pub fn new(value: T) -> Self { From a5da003cad329f4ee941df677ae9a5caaeddc9cc Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Tue, 23 Mar 2021 21:51:46 +0000 Subject: [PATCH 12/21] Fix memory leak and discuss implications Based on [a comment][comment] by danielhenrymantilla [comment]: https://github.com/rust-lang/nomicon/pull/259#discussion_r599950517 --- src/send-and-sync.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index ee161abe..d7d423c6 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -206,6 +206,31 @@ sort of Box would be Send, which in this case is the same as saying T is Send. unsafe impl Send for Carton where Box: Send {} ``` +Right now Carton has a memory leak, as it never frees the memory it allocates. +Once we fix that we have a new requirement we have to ensure we meet to be Send: +we need to know `free` can be called on a pointer that was yielded by an +allocation done on another thread. This is the case for `libc::free`, so we can +still be Send. + +```rust,ignore +impl Drop for Carton { + fn drop(&mut self) { + unsafe { + libc::free(self.0.as_ptr().cast()); + } + } +} +``` + +A nice example where this does not happen is with a MutexGuard: notice how +[it is not Send][mutex-guard-not-send-docs-rs]. The implementation of MutexGuard +[uses libraries][mutex-guard-not-send-comment] that require you to ensure you +don't try to free a lock that you acquired in a different thread. If you were +able to Send a MutexGuard to another thread the destructor would run in the +thread you sent it to, violating the requirement. MutexGuard can still be sync +because all you can send to another thread is an `&MutexGuard` and dropping a +reference does nothing. + TODO: better explain what can or can't be Send or Sync. Sufficient to appeal only to data races? @@ -214,3 +239,5 @@ only to data races? [box-is-special]: https://manishearth.github.io/blog/2017/01/10/rust-tidbits-box-is-special/ [deref-doc]: https://doc.rust-lang.org/core/ops/trait.Deref.html [deref-mut-doc]: https://doc.rust-lang.org/core/ops/trait.DerefMut.html +[mutex-guard-not-send-docs-rs]: https://doc.rust-lang.org/std/sync/struct.MutexGuard.html#impl-Send +[mutex-guard-not-send-comment]: https://github.com/rust-lang/rust/issues/23465#issuecomment-82730326 From d060a51864448782479a7de578675a5f6bb093e8 Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Tue, 23 Mar 2021 22:00:10 +0000 Subject: [PATCH 13/21] Reword --- src/send-and-sync.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index d7d423c6..90a2a0a6 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -209,8 +209,8 @@ unsafe impl Send for Carton where Box: Send {} Right now Carton has a memory leak, as it never frees the memory it allocates. Once we fix that we have a new requirement we have to ensure we meet to be Send: we need to know `free` can be called on a pointer that was yielded by an -allocation done on another thread. This is the case for `libc::free`, so we can -still be Send. +allocation done on another thread. We can check this is true in the docs for +[`libc::free`][libc-free-docs]. ```rust,ignore impl Drop for Carton { @@ -227,7 +227,7 @@ A nice example where this does not happen is with a MutexGuard: notice how [uses libraries][mutex-guard-not-send-comment] that require you to ensure you don't try to free a lock that you acquired in a different thread. If you were able to Send a MutexGuard to another thread the destructor would run in the -thread you sent it to, violating the requirement. MutexGuard can still be sync +thread you sent it to, violating the requirement. MutexGuard can still be Sync because all you can send to another thread is an `&MutexGuard` and dropping a reference does nothing. @@ -241,3 +241,4 @@ only to data races? [deref-mut-doc]: https://doc.rust-lang.org/core/ops/trait.DerefMut.html [mutex-guard-not-send-docs-rs]: https://doc.rust-lang.org/std/sync/struct.MutexGuard.html#impl-Send [mutex-guard-not-send-comment]: https://github.com/rust-lang/rust/issues/23465#issuecomment-82730326 +[libc-free-docs]: https://linux.die.net/man/3/free From 09d4be20f4ee1b83ab2ec65de805e0523c2d9025 Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Tue, 23 Mar 2021 22:30:12 +0000 Subject: [PATCH 14/21] Update src/send-and-sync.md Co-authored-by: Daniel Henry-Mantilla --- src/send-and-sync.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index 90a2a0a6..bc75ea12 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -93,7 +93,7 @@ struct Carton(ptr::NonNull); impl Carton { pub fn new(value: T) -> Self { // Allocate enough memory on the heap to store one T. - assert_ne!(size_of::(), 0, "Zero-sized types are out of the scope of this example); + assert_ne!(size_of::(), 0, "Zero-sized types are out of the scope of this example"); let memptr: *mut c_void = ptr::null_mut(); unsafe { let ret = libc::posix_memalign( From 484bbeba1e4fe13ad815329d3e64d34e46967266 Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Thu, 25 Mar 2021 20:06:18 +0000 Subject: [PATCH 15/21] Update src/send-and-sync.md Co-authored-by: Daniel Henry-Mantilla --- src/send-and-sync.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index bc75ea12..97e8b244 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -108,7 +108,7 @@ impl Carton { let mut ptr = unsafe { // Safety: memptr is dereferenceable because we created it from a // reference and have exclusive access. - NonNull::new(memptr.cast::()) + ptr::NonNull::new(memptr.cast::()) .expect("Guaranteed non-null if posix_memalign returns 0") }; From d1c89a828dabf0719fc3061b60bc4b48752c5b98 Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Thu, 25 Mar 2021 20:06:24 +0000 Subject: [PATCH 16/21] Update src/send-and-sync.md Co-authored-by: Daniel Henry-Mantilla --- src/send-and-sync.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index 97e8b244..1e2bb046 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -84,10 +84,16 @@ implement Send and Sync. Let's call it a `Carton`. We start by writing code to take a value allocated on the stack and transfer it to the heap. -```rust,ignore -use std::mem::size_of; -use std::ptr; - +```rust +# mod libc { +# pub use ::std::os::raw::{c_int, c_void, size_t}; +# extern "C" { pub fn posix_memalign(memptr: *mut *mut c_void, align: size_t, size: size_t) -> c_int; } +# } +use std::{ + mem::{align_of, size_of}, + ptr, +}; +use libc::c_void; struct Carton(ptr::NonNull); impl Carton { From 7a1be9d5b0fcc5588009194dac73870d4b173a37 Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Thu, 25 Mar 2021 20:44:14 +0000 Subject: [PATCH 17/21] Fix failing tests --- src/send-and-sync.md | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index 1e2bb046..0fa89e00 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -85,25 +85,27 @@ We start by writing code to take a value allocated on the stack and transfer it to the heap. ```rust -# mod libc { -# pub use ::std::os::raw::{c_int, c_void, size_t}; -# extern "C" { pub fn posix_memalign(memptr: *mut *mut c_void, align: size_t, size: size_t) -> c_int; } +# pub mod libc { +# pub use ::std::os::raw::{c_int, c_void}; +# #[allow(non_camel_case_types)] +# pub type size_t = usize; +# extern "C" { pub fn posix_memalign(memptr: *mut *mut c_void, align: size_t, size: size_t) -> c_int; } # } use std::{ mem::{align_of, size_of}, ptr, }; -use libc::c_void; + struct Carton(ptr::NonNull); impl Carton { pub fn new(value: T) -> Self { // Allocate enough memory on the heap to store one T. assert_ne!(size_of::(), 0, "Zero-sized types are out of the scope of this example"); - let memptr: *mut c_void = ptr::null_mut(); + let mut memptr = ptr::null_mut() as *mut T; unsafe { let ret = libc::posix_memalign( - &mut memptr, + (&mut memptr).cast(), align_of::(), size_of::() ); @@ -218,7 +220,12 @@ we need to know `free` can be called on a pointer that was yielded by an allocation done on another thread. We can check this is true in the docs for [`libc::free`][libc-free-docs]. -```rust,ignore +```rust +# struct Carton(std::ptr::NonNull); +# mod libc { +# pub use ::std::os::raw::c_void; +# extern "C" { pub fn free(p: *mut c_void); } +# } impl Drop for Carton { fn drop(&mut self) { unsafe { From 33fa3de81090d025de7ae593696d56a54fd57c63 Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Thu, 1 Apr 2021 13:42:22 +0100 Subject: [PATCH 18/21] Update src/send-and-sync.md Co-authored-by: Yuki Okushi --- src/send-and-sync.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index 0fa89e00..2e387e91 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -141,7 +141,7 @@ that. use std::ops::{Deref, DerefMut}; # struct Carton(std::ptr::NonNull); - +# impl Deref for Carton { type Target = T; From 4a910d8e6566df24f96e13188dc957ef9e8b9344 Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Thu, 1 Apr 2021 13:42:34 +0100 Subject: [PATCH 19/21] Update src/send-and-sync.md Co-authored-by: Yuki Okushi --- src/send-and-sync.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index 2e387e91..f7ff82f1 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -162,7 +162,7 @@ impl DerefMut for Carton { fn deref_mut(&mut self) -> &mut Self::Target { unsafe { // Safety: The pointer is aligned, initialized, and dereferenceable - // by the logic in [`Self::new]. We require writers to mutably + // by the logic in [`Self::new`]. We require writers to mutably // borrow the Carton, and the lifetime of the return value is // elided to the lifetime of the input. This means the borrow // checker will enforce that no one else can access the contents From 047dc6bcf170ff2993a34b0617fa257d4d50b38c Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Thu, 1 Apr 2021 13:42:45 +0100 Subject: [PATCH 20/21] Update src/send-and-sync.md Co-authored-by: Yuki Okushi --- src/send-and-sync.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index f7ff82f1..67858e77 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -173,7 +173,7 @@ impl DerefMut for Carton { } ``` -Finally, lets think about whether our `Carton` is Send and Sync. Something can +Finally, let's think about whether our `Carton` is Send and Sync. Something can safely be Send unless it shares mutable state with something else without enforcing exclusive access to it. Each `Carton` has a unique pointer, so we're good. From 64957019c86a14549113aaae39c4b613225191de Mon Sep 17 00:00:00 2001 From: Daniel Franklin Date: Thu, 1 Apr 2021 13:42:55 +0100 Subject: [PATCH 21/21] Update src/send-and-sync.md Co-authored-by: Yuki Okushi --- src/send-and-sync.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/send-and-sync.md b/src/send-and-sync.md index 67858e77..97423282 100644 --- a/src/send-and-sync.md +++ b/src/send-and-sync.md @@ -214,7 +214,7 @@ sort of Box would be Send, which in this case is the same as saying T is Send. unsafe impl Send for Carton where Box: Send {} ``` -Right now Carton has a memory leak, as it never frees the memory it allocates. +Right now `Carton` has a memory leak, as it never frees the memory it allocates. Once we fix that we have a new requirement we have to ensure we meet to be Send: we need to know `free` can be called on a pointer that was yielded by an allocation done on another thread. We can check this is true in the docs for