From c5258d30a6025278b13eb365bbaed6a26e8b142e Mon Sep 17 00:00:00 2001 From: Jeb Bearer Date: Tue, 2 Jul 2024 10:53:42 -0400 Subject: [PATCH] Replace unsafe projection code with safe derive macro --- Cargo.lock | 1 + Cargo.toml | 1 + src/socket.rs | 53 ++++----------------------------------------------- 3 files changed, 6 insertions(+), 49 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a99f1f2..b10e764 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3489,6 +3489,7 @@ dependencies = [ "markdown", "maud", "parking_lot", + "pin-project", "portpicker", "prometheus", "reqwest", diff --git a/Cargo.toml b/Cargo.toml index 048d3e1..90b0ce7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,7 @@ libc = "0.2" markdown = "0.3" maud = { version = "0.26", features = ["tide"] } parking_lot = "0.12" +pin-project = "1.0" prometheus = "0.13" reqwest = { version = "0.12", features = ["json"] } routefinder = "0.5" diff --git a/src/socket.rs b/src/socket.rs index 13a4952..c685940 100644 --- a/src/socket.rs +++ b/src/socket.rs @@ -20,6 +20,7 @@ use futures::{ task::{Context, Poll}, FutureExt, Sink, SinkExt, Stream, StreamExt, TryFutureExt, }; +use pin_project::pin_project; use serde::{de::DeserializeOwned, Serialize}; use std::borrow::Cow; use std::fmt::{self, Display, Formatter}; @@ -133,7 +134,9 @@ enum MessageType { /// /// [Connection] implements [Stream], which can be used to receive `FromClient` messages from the /// client, and [Sink] which can be used to send `ToClient` messages to the client. +#[pin_project] pub struct Connection { + #[pin] conn: WebSocketConnection, // [Sink] wrapper around `conn` sink: Pin>>>, @@ -150,7 +153,7 @@ impl fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { // Get a `Pin<&mut WebSocketConnection>` for the underlying connection, so we can use the // `Stream` implementation of that field. - match self.pinned_inner().poll_next(cx) { + match self.project().conn.poll_next(cx) { Poll::Ready(None) => Poll::Ready(None), Poll::Ready(Some(Err(err))) => Poll::Ready(Some(Err(err.into()))), Poll::Ready(Some(Ok(msg))) => Poll::Ready(Some(match msg { @@ -216,32 +219,6 @@ impl Sink } } -impl Drop - for Connection -{ - fn drop(&mut self) { - // This is the idiomatic way to implement [drop] for a type that uses pinning. Since [drop] - // is implicitly called with `&mut self` even on types that were pinned, we place any - // implementation inside [inner_drop], which takes `Pin<&mut Self>`, when the commpiler will - // be able to check that we do not do anything that we couldn't have done on a - // `Pin<&mut Self>`. - // - // The [drop] implementation for this type is trivial, and it would be safe to use the - // automatically generated [drop] implementation, but we nonetheless implement [drop] - // explicitly in the idiomatic fashion so that it is impossible to accidentally implement an - // unsafe version of [drop] for this type in the future. - - // `new_unchecked` is okay because we know this value is never used again after being - // dropped. - inner_drop(unsafe { Pin::new_unchecked(self) }); - fn inner_drop( - _this: Pin<&mut Connection>, - ) { - // Any logic goes here. - } - } -} - impl Connection { @@ -271,28 +248,6 @@ impl Ok(conn) })) } - - /// Project a `Pin<&mut Self>` to a pinned reference to the underlying connection. - fn pinned_inner(self: Pin<&mut Self>) -> Pin<&mut WebSocketConnection> { - // # Soundness - // - // This implements _structural pinning_ for [Connection]. This comes with some requirements - // to maintain safety, as described at - // https://doc.rust-lang.org/std/pin/index.html#pinning-is-structural-for-field: - // - // 1. The struct must only be [Unpin] if all the structural fields are [Unpin]. This is the - // default, and we don't explicitly implement [Unpin] for [Connection]. - // 2. The destructor of the struct must not move structural fields out of its argument. This - // is enforced by the compiler in our [Drop] implementation, which follows the idiom for - // safe [Drop] implementations for pinned structs. - // 3. You must make sure that you uphold the [Drop] guarantee: once your struct is pinned, - // the memory that contains the content is not overwritten or deallocated without calling - // the content’s destructors. This is also enforced by our [Drop] implementation. - // 4. You must not offer any other operations that could lead to data being moved out of the - // structural fields when your type is pinned. There are no operations on this type that - // move out of `conn`. - unsafe { self.map_unchecked_mut(|s| &mut s.conn) } - } } impl Clone