Remove the unsound custom Cell and replace it with Rc<RefCell<T> - the way it was before the introduction of a custom cell.

Heavily based on the patch by Wim Looman <wim@nemo157.com> https://gist.github.com/Nemo157/b495b84b5b2f3134d19ad71ba5002066

actix-service/src/and_then.rs

@@ -1,16 +1,17 @@
 use std::future::Future;
 use std::pin::Pin;
 use std::rc::Rc;
+use std::cell::RefCell;
 use std::task::{Context, Poll};
 
 use super::{Service, ServiceFactory};
-use crate::cell::Cell;
+
 
 /// Service for the `and_then` combinator, chaining a computation onto the end
 /// of another service which completes successfully.
 ///
 /// This is created by the `ServiceExt::and_then` method.
-pub(crate) struct AndThenService<A, B>(Cell<(A, B)>);
+pub(crate) struct AndThenService<A, B>(Rc<RefCell<(A, B)>>);
 
 impl<A, B> AndThenService<A, B> {
     /// Create new `AndThen` combinator
@@ -19,7 +20,7 @@ impl<A, B> AndThenService<A, B> {
         A: Service,
         B: Service<Request = A::Response, Error = A::Error>,
     {
-        Self(Cell::new((a, b)))
+        Self(Rc::new(RefCell::new((a, b))))
     }
 }
 
@@ -40,7 +41,7 @@ where
     type Future = AndThenServiceResponse<A, B>;
 
     fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
-        let srv = self.0.get_mut();
+        let mut srv = self.0.borrow_mut();
         let not_ready = !srv.0.poll_ready(cx)?.is_ready();
         if !srv.1.poll_ready(cx)?.is_ready() || not_ready {
             Poll::Pending
@@ -51,7 +52,7 @@ where
 
     fn call(&mut self, req: A::Request) -> Self::Future {
         AndThenServiceResponse {
-            state: State::A(self.0.get_mut().0.call(req), Some(self.0.clone())),
+            state: State::A(self.0.borrow_mut().0.call(req), Some(self.0.clone())),
         }
     }
 }
@@ -72,7 +73,7 @@ where
     A: Service,
     B: Service<Request = A::Response, Error = A::Error>,
 {
-    A(#[pin] A::Future, Option<Cell<(A, B)>>),
+    A(#[pin] A::Future, Option<Rc<RefCell<(A, B)>>>),
     B(#[pin] B::Future),
     Empty,
 }
@@ -90,9 +91,9 @@ where
         match this.state.as_mut().project() {
             StateProj::A(fut, b) => match fut.poll(cx)? {
                 Poll::Ready(res) => {
-                    let mut b = b.take().unwrap();
+                    let b = b.take().unwrap();
                     this.state.set(State::Empty); // drop fut A
-                    let fut = b.get_mut().1.call(res);
+                    let fut = b.borrow_mut().1.call(res);
                     this.state.set(State::B(fut));
                     self.poll(cx)
                 }

actix-service/src/and_then_apply_fn.rs

@@ -2,9 +2,9 @@ use std::future::Future;
 use std::marker::PhantomData;
 use std::pin::Pin;
 use std::rc::Rc;
+use std::cell::RefCell;
 use std::task::{Context, Poll};
 
-use crate::cell::Cell;
 use crate::{Service, ServiceFactory};
 
 /// `Apply` service combinator
@@ -16,7 +16,7 @@ where
     Fut: Future<Output = Result<Res, Err>>,
     Err: From<A::Error> + From<B::Error>,
 {
-    srv: Cell<(A, B, F)>,
+    srv: Rc<RefCell<(A, B, F)>>,
     r: PhantomData<(Fut, Res, Err)>,
 }
 
@@ -31,7 +31,7 @@ where
     /// Create new `Apply` combinator
     pub(crate) fn new(a: A, b: B, f: F) -> Self {
         Self {
-            srv: Cell::new((a, b, f)),
+            srv:  Rc::new(RefCell::new((a, b, f))),
             r: PhantomData,
         }
     }
@@ -67,7 +67,7 @@ where
     type Future = AndThenApplyFnFuture<A, B, F, Fut, Res, Err>;
 
     fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
-        let inner = self.srv.get_mut();
+        let mut inner = self.srv.borrow_mut();
         let not_ready = inner.0.poll_ready(cx)?.is_pending();
         if inner.1.poll_ready(cx)?.is_pending() || not_ready {
             Poll::Pending
@@ -77,7 +77,7 @@ where
     }
 
     fn call(&mut self, req: A::Request) -> Self::Future {
-        let fut = self.srv.get_mut().0.call(req);
+        let fut = self.srv.borrow_mut().0.call(req);
         AndThenApplyFnFuture {
             state: State::A(fut, Some(self.srv.clone())),
         }
@@ -108,7 +108,7 @@ where
     Err: From<A::Error>,
     Err: From<B::Error>,
 {
-    A(#[pin] A::Future, Option<Cell<(A, B, F)>>),
+    A(#[pin] A::Future, Option<Rc<RefCell<(A, B, F)>>>),
     B(#[pin] Fut),
     Empty,
 }
@@ -129,10 +129,10 @@ where
         match this.state.as_mut().project() {
             StateProj::A(fut, b) => match fut.poll(cx)? {
                 Poll::Ready(res) => {
-                    let mut b = b.take().unwrap();
+                    let b = b.take().unwrap();
                     this.state.set(State::Empty);
-                    let b = b.get_mut();
+                    let (_, b, f) = &mut *b.borrow_mut();
-                    let fut = (&mut b.2)(res, &mut b.1);
+                    let fut = f(res, b);
                     this.state.set(State::B(fut));
                     self.poll(cx)
                 }
@@ -255,11 +255,11 @@ where
 
         if this.a.is_some() && this.b.is_some() {
             Poll::Ready(Ok(AndThenApplyFn {
-                srv: Cell::new((
+                srv: Rc::new(RefCell::new((
                     this.a.take().unwrap(),
                     this.b.take().unwrap(),
                     this.f.clone(),
-                )),
+                ))),
                 r: PhantomData,
             }))
         } else {

actix-service/src/apply_cfg.rs

@@ -2,8 +2,9 @@ use std::future::Future;
 use std::marker::PhantomData;
 use std::pin::Pin;
 use std::task::{Context, Poll};
+use std::rc::Rc;
+use std::cell::RefCell;
 
-use crate::cell::Cell;
 use crate::{Service, ServiceFactory};
 
 /// Convert `Fn(Config, &mut Service1) -> Future<Service2>` fn to a service factory
@@ -26,7 +27,7 @@ where
     S: Service,
 {
     ApplyConfigService {
-        srv: Cell::new((srv, f)),
+        srv: Rc::new(RefCell::new((srv, f))),
         _t: PhantomData,
     }
 }
@@ -53,7 +54,7 @@ where
     S: Service,
 {
     ApplyConfigServiceFactory {
-        srv: Cell::new((factory, f)),
+        srv: Rc::new(RefCell::new((factory, f))),
         _t: PhantomData,
     }
 }
@@ -66,7 +67,7 @@ where
     R: Future<Output = Result<S, E>>,
     S: Service,
 {
-    srv: Cell<(T, F)>,
+    srv: Rc<RefCell<(T, F)>>,
     _t: PhantomData<(C, R, S)>,
 }
 
@@ -102,10 +103,8 @@ where
     type Future = R;
 
     fn new_service(&self, cfg: C) -> Self::Future {
-        unsafe {
+        let (t, f) = &mut *self.srv.borrow_mut();
-            let srv = self.srv.get_mut_unsafe();
+        f(cfg, t)
-            (srv.1)(cfg, &mut srv.0)
-        }
     }
 }
 
@@ -117,7 +116,7 @@ where
     R: Future<Output = Result<S, T::InitError>>,
     S: Service,
 {
-    srv: Cell<(T, F)>,
+    srv: Rc<RefCell<(T, F)>>,
     _t: PhantomData<(C, R, S)>,
 }
 
@@ -157,7 +156,7 @@ where
         ApplyConfigServiceFactoryResponse {
             cfg: Some(cfg),
             store: self.srv.clone(),
-            state: State::A(self.srv.get_ref().0.new_service(())),
+            state: State::A(self.srv.borrow().0.new_service(())),
         }
     }
 }
@@ -172,7 +171,7 @@ where
     S: Service,
 {
     cfg: Option<C>,
-    store: Cell<(T, F)>,
+    store: Rc<RefCell<(T, F)>>,
     #[pin]
     state: State<T, R, S>,
 }
@@ -213,8 +212,11 @@ where
             },
             StateProj::B(srv) => match srv.poll_ready(cx)? {
                 Poll::Ready(_) => {
-                    let fut = (this.store.get_mut().1)(this.cfg.take().unwrap(), srv);
+                    {
-                    this.state.set(State::C(fut));
+                        let (_, f) = &mut *this.store.borrow_mut();
+                        let fut = f(this.cfg.take().unwrap(), srv);
+                        this.state.set(State::C(fut));
+                    }
                     self.poll(cx)
                 }
                 Poll::Pending => Poll::Pending,

actix-service/src/cell.rs

@@ -1,57 +0,0 @@
-//! Custom cell impl, internal use only
-use std::task::{Context, Poll};
-use std::{cell::UnsafeCell, fmt, rc::Rc};
-
-pub(crate) struct Cell<T> {
-    inner: Rc<UnsafeCell<T>>,
-}
-
-impl<T> Clone for Cell<T> {
-    fn clone(&self) -> Self {
-        Self {
-            inner: self.inner.clone(),
-        }
-    }
-}
-
-impl<T: fmt::Debug> fmt::Debug for Cell<T> {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.inner.fmt(f)
-    }
-}
-
-impl<T> Cell<T> {
-    pub(crate) fn new(inner: T) -> Self {
-        Self {
-            inner: Rc::new(UnsafeCell::new(inner)),
-        }
-    }
-
-    pub(crate) fn get_ref(&self) -> &T {
-        unsafe { &*self.inner.as_ref().get() }
-    }
-
-    pub(crate) fn get_mut(&mut self) -> &mut T {
-        unsafe { &mut *self.inner.as_ref().get() }
-    }
-
-    #[allow(clippy::mut_from_ref)]
-    pub(crate) unsafe fn get_mut_unsafe(&self) -> &mut T {
-        &mut *self.inner.as_ref().get()
-    }
-}
-
-impl<T: crate::Service> crate::Service for Cell<T> {
-    type Request = T::Request;
-    type Response = T::Response;
-    type Error = T::Error;
-    type Future = T::Future;
-
-    fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
-        self.get_mut().poll_ready(cx)
-    }
-
-    fn call(&mut self, req: Self::Request) -> Self::Future {
-        self.get_mut().call(req)
-    }
-}

actix-service/src/lib.rs

@@ -12,7 +12,6 @@ mod and_then_apply_fn;
 mod apply;
 mod apply_cfg;
 pub mod boxed;
-mod cell;
 mod fn_service;
 mod map;
 mod map_config;

actix-service/src/then.rs

@@ -1,16 +1,16 @@
 use std::future::Future;
 use std::pin::Pin;
 use std::rc::Rc;
+use std::cell::RefCell;
 use std::task::{Context, Poll};
 
 use super::{Service, ServiceFactory};
-use crate::cell::Cell;
 
 /// Service for the `then` combinator, chaining a computation onto the end of
 /// another service.
 ///
 /// This is created by the `Pipeline::then` method.
-pub(crate) struct ThenService<A, B>(Cell<(A, B)>);
+pub(crate) struct ThenService<A, B>(Rc<RefCell<(A, B)>>);
 
 impl<A, B> ThenService<A, B> {
     /// Create new `.then()` combinator
@@ -19,7 +19,7 @@ impl<A, B> ThenService<A, B> {
         A: Service,
         B: Service<Request = Result<A::Response, A::Error>, Error = A::Error>,
     {
-        Self(Cell::new((a, b)))
+        Self(Rc::new(RefCell::new((a, b))))
     }
 }
 
@@ -40,7 +40,7 @@ where
     type Future = ThenServiceResponse<A, B>;
 
     fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
-        let srv = self.0.get_mut();
+        let mut srv = self.0.borrow_mut();
         let not_ready = !srv.0.poll_ready(cx)?.is_ready();
         if !srv.1.poll_ready(cx)?.is_ready() || not_ready {
             Poll::Pending
@@ -51,7 +51,7 @@ where
 
     fn call(&mut self, req: A::Request) -> Self::Future {
         ThenServiceResponse {
-            state: State::A(self.0.get_mut().0.call(req), Some(self.0.clone())),
+            state: State::A(self.0.borrow_mut().0.call(req), Some(self.0.clone())),
         }
     }
 }
@@ -72,7 +72,7 @@ where
     A: Service,
     B: Service<Request = Result<A::Response, A::Error>>,
 {
-    A(#[pin] A::Future, Option<Cell<(A, B)>>),
+    A(#[pin] A::Future, Option<Rc<RefCell<(A, B)>>>),
     B(#[pin] B::Future),
     Empty,
 }
@@ -90,9 +90,9 @@ where
         match this.state.as_mut().project() {
             StateProj::A(fut, b) => match fut.poll(cx) {
                 Poll::Ready(res) => {
-                    let mut b = b.take().unwrap();
+                    let b = b.take().unwrap();
                     this.state.set(State::Empty); // drop fut A
-                    let fut = b.get_mut().1.call(res);
+                    let fut = b.borrow_mut().1.call(res);
                     this.state.set(State::B(fut));
                     self.poll(cx)
                 }