From 16cab308f2c722d7e2474655605adf7eb87a03b8 Mon Sep 17 00:00:00 2001
From: Jake Hughes <jh@jakehughes.uk>
Date: Sat, 6 Feb 2021 20:09:15 +0000
Subject: [PATCH] Make Gc<T> require bounds `T: Send + Sync`

The collector finalizes objects off the main thread. This means that if
any fields are accessed via T's drop method, it must be done in a
thread-safe way.

A common pattern is to allow the `Gc<T>` to hold trait objects. For this
change to work, TO methods need a `where Self: Send + Sync` clause for
the compiler to guarantee object safety. Such clauses are deprecated in
rustc because they can be used to create UB when binding to user traits
with methods. However, at the time of this commit it's the only known
way to add additional bounds for auto traits. From my understanding this
usage is considered safe until a better situation arises [1].

[1]: https://github.com/rust-lang/rust/issues/50781#issuecomment-393615608
---
 src/gc.rs  | 49 ++++++++++++++++++++++++++++++-------------------
 src/lib.rs |  1 +
 2 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/src/gc.rs b/src/gc.rs
index e4d3fc7..e85d621 100644
--- a/src/gc.rs
+++ b/src/gc.rs
@@ -39,15 +39,18 @@ pub fn gc_init() {
 /// `Gc<T>` automatically dereferences to `T` (via the `Deref` trait), so
 /// you can call `T`'s methods on a value of type `Gc<T>`.
 #[derive(PartialEq, Eq, Debug)]
-pub struct Gc<T: ?Sized> {
+pub struct Gc<T: ?Sized + Send + Sync> {
     ptr: NonNull<GcBox<T>>,
     _phantom: PhantomData<T>,
 }
 
-impl<T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<Gc<U>> for Gc<T> {}
-impl<T: ?Sized + Unsize<U>, U: ?Sized> DispatchFromDyn<Gc<U>> for Gc<T> {}
+impl<T: ?Sized + Unsize<U> + Send + Sync, U: ?Sized + Send + Sync> CoerceUnsized<Gc<U>> for Gc<T> {}
+impl<T: ?Sized + Unsize<U> + Send + Sync, U: ?Sized + Send + Sync> DispatchFromDyn<Gc<U>>
+    for Gc<T>
+{
+}
 
-impl<T> Gc<T> {
+impl<T: Send + Sync> Gc<T> {
     /// Constructs a new `Gc<T>`.
     pub fn new(v: T) -> Self {
         Gc {
@@ -101,8 +104,8 @@ impl<T> Gc<T> {
     }
 }
 
-impl Gc<dyn Any> {
-    pub fn downcast<T: Any>(&self) -> Result<Gc<T>, Gc<dyn Any>> {
+impl Gc<dyn Any + Send + Sync> {
+    pub fn downcast<T: Any + Send + Sync>(&self) -> Result<Gc<T>, Gc<dyn Any + Send + Sync>> {
         if (*self).is::<T>() {
             let ptr = self.ptr.cast::<GcBox<T>>();
             Ok(Gc::from_inner(ptr))
@@ -122,7 +125,7 @@ pub fn needs_finalizer<T>() -> bool {
     std::mem::needs_finalizer::<T>()
 }
 
-impl<T: ?Sized> Gc<T> {
+impl<T: ?Sized + Send + Sync> Gc<T> {
     /// Get a raw pointer to the underlying value `T`.
     pub fn into_raw(this: Self) -> *const T {
         this.ptr.as_ptr() as *const T
@@ -156,7 +159,7 @@ impl<T: ?Sized> Gc<T> {
     }
 }
 
-impl<T> Gc<MaybeUninit<T>> {
+impl<T: Send + Sync> Gc<MaybeUninit<T>> {
     /// As with `MaybeUninit::assume_init`, it is up to the caller to guarantee
     /// that the inner value really is in an initialized state. Calling this
     /// when the content is not yet fully initialized causes immediate undefined
@@ -167,7 +170,7 @@ impl<T> Gc<MaybeUninit<T>> {
     }
 }
 
-impl<T: ?Sized + fmt::Display> fmt::Display for Gc<T> {
+impl<T: ?Sized + fmt::Display + Send + Sync> fmt::Display for Gc<T> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt::Display::fmt(&**self, f)
     }
@@ -239,7 +242,7 @@ impl<T> GcBox<MaybeUninit<T>> {
     }
 }
 
-impl<T: ?Sized> Deref for Gc<T> {
+impl<T: ?Sized + Send + Sync> Deref for Gc<T> {
     type Target = T;
 
     fn deref(&self) -> &Self::Target {
@@ -250,15 +253,15 @@ impl<T: ?Sized> Deref for Gc<T> {
 /// `Copy` and `Clone` are implemented manually because a reference to `Gc<T>`
 /// should be copyable regardless of `T`. It differs subtly from `#[derive(Copy,
 /// Clone)]` in that the latter only makes `Gc<T>` copyable if `T` is.
-impl<T: ?Sized> Copy for Gc<T> {}
+impl<T: ?Sized + Send + Sync> Copy for Gc<T> {}
 
-impl<T: ?Sized> Clone for Gc<T> {
+impl<T: ?Sized + Send + Sync> Clone for Gc<T> {
     fn clone(&self) -> Self {
         *self
     }
 }
 
-impl<T: ?Sized + Hash> Hash for Gc<T> {
+impl<T: ?Sized + Hash + Send + Sync> Hash for Gc<T> {
     fn hash<H: Hasher>(&self, state: &mut H) {
         (**self).hash(state);
     }
@@ -293,16 +296,24 @@ mod test {
         struct S2 {
             y: u64,
         }
-        trait T {
-            fn f(self: Gc<Self>) -> u64;
+        trait T: Send + Sync {
+            fn f(self: Gc<Self>) -> u64
+            where
+                Self: Send + Sync;
         }
         impl T for S1 {
-            fn f(self: Gc<Self>) -> u64 {
+            fn f(self: Gc<Self>) -> u64
+            where
+                Self: Send + Sync,
+            {
                 self.x
             }
         }
         impl T for S2 {
-            fn f(self: Gc<Self>) -> u64 {
+            fn f(self: Gc<Self>) -> u64
+            where
+                Self: Send + Sync,
+            {
                 self.y
             }
         }
@@ -314,8 +325,8 @@ mod test {
         assert_eq!(s1gc.f(), 1);
         assert_eq!(s2gc.f(), 2);
 
-        let s1gcd: Gc<T> = s1gc;
-        let s2gcd: Gc<T> = s2gc;
+        let s1gcd: Gc<dyn T> = s1gc;
+        let s2gcd: Gc<dyn T> = s2gc;
         assert_eq!(s1gcd.f(), 1);
         assert_eq!(s2gcd.f(), 2);
     }
diff --git a/src/lib.rs b/src/lib.rs
index a3e1124..2906415 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -12,6 +12,7 @@
 #![feature(maybe_uninit_ref)]
 #![feature(negative_impls)]
 #![allow(incomplete_features)]
+#![allow(where_clauses_object_safety)]
 #[cfg(not(all(target_pointer_width = "64", target_arch = "x86_64")))]
 compile_error!("Requires x86_64 with 64 bit pointer width.");