feat(bindings): deprecate & alias Connection::wipe

bindings/rust/extended/s2n-tls-tokio/tests/handshake.rs

@@ -2,12 +2,13 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use rand::Rng;
+#[allow(deprecated)]
+use s2n_tls::pool::ConfigPoolBuilder;
 use s2n_tls::{
     config::Config,
     connection::{Connection, ModifiedBuilder},
     enums::{ClientAuthType, Mode, Version},
     error::{Error, ErrorType},
-    pool::ConfigPoolBuilder,
     security::{DEFAULT_TLS13, TESTING_TLS12},
 };
 use s2n_tls_tokio::{TlsAcceptor, TlsConnector};
@@ -45,6 +46,7 @@ async fn handshake_basic() -> Result<(), Box<dyn std::error::Error>> {
 }
 
 #[tokio::test(flavor = "multi_thread")]
+#[allow(deprecated)]
 async fn handshake_with_pool_multithread() -> Result<(), Box<dyn std::error::Error>> {
     const COUNT: usize = 20;
     const CLIENT_LIMIT: usize = 3;
@@ -109,6 +111,7 @@ async fn handshake_with_connection_config() -> Result<(), Box<dyn std::error::Er
 }
 
 #[tokio::test]
+#[allow(deprecated)]
 async fn handshake_with_connection_config_with_pool() -> Result<(), Box<dyn std::error::Error>> {
     fn with_client_auth(conn: &mut Connection) -> Result<&mut Connection, Error> {
         conn.set_client_auth_type(ClientAuthType::Optional)

bindings/rust/extended/s2n-tls/src/connection.rs

@@ -558,6 +558,7 @@ impl Connection {
         Ok(self)
     }
 
+    #[cfg(feature = "unstable-renegotiate")]
     pub(crate) fn wipe_method<F, T>(&mut self, wipe: F) -> Result<(), Error>
     where
         F: FnOnce(&mut Self) -> Result<T, Error>,
@@ -575,19 +576,31 @@ impl Connection {
         Ok(())
     }
 
-    /// wipes an existing connection and allows it to be reused.
+    /// Resets a connection so that it can be reused.
     ///
-    /// This method erases all data associated with a connection including pending reads.
-    /// This function should be called after all I/O is completed and s2n_shutdown has been
-    /// called. Reusing the same connection handle(s) is more performant than repeatedly
-    /// calling s2n_connection_new and s2n_connection_free
+    /// This method no longer wipes the existing connection. Instead, it replaces the
+    /// connection with a newly allocated one, preserving the mode and config.
     ///
-    /// Corresponds to [`s2n_connection_wipe`].
+    /// This method should be called after all I/O is completed and `Connection::poll_shutdown`
+    /// has been called.
+    #[deprecated(
+        note = "use `Connection::new()` instead; connection reuse provides negligible performance benefit"
+    )]
     pub fn wipe(&mut self) -> Result<&mut Self, Error> {
-        self.wipe_method(|conn| unsafe { s2n_connection_wipe(conn.as_ptr()).into_result() })?;
-        // we deliberately call this outside of "wipe_method", because binding
-        // specific defaults should not be re-applied on renegotiate wipe
-        self.set_binding_specific_defaults()?;
+        // s2n_connection_wipe is a nightmare of a method, with lifetime issues
+        // that are incredibly difficult to reason about. We do not expose it in
+        // the rust bindings. In our benchmarking, the savings were ~ 2 us, which
+        // is less than 1% of the cost of a handshake.
+        let clean_connection = {
+            let mut connection = Connection::new(self.mode());
+            // config will be none if Connection::set_config has yet to be called
+            if let Some(config) = self.config() {
+                connection.set_config(config)?;
+            }
+            connection
+        };
+        *self = clean_connection;
+
         Ok(self)
     }
 
@@ -1975,6 +1988,7 @@ mod tests {
     /// Confirm that the large (16KB) record size is used by both newly
     /// created connections and wiped (reused) connections.
     #[test]
+    #[allow(deprecated)]
     fn max_record_size_configuration() -> Result<(), Box<dyn std::error::Error>> {
         /// https://www.rfc-editor.org/info/rfc8446/#section-5.1
         /// > The length MUST NOT exceed 2^14 bytes.
@@ -2070,6 +2084,7 @@ mod tests {
 
     /// `wipe` preserves the mode (client/server) of the connection.
     #[test]
+    #[allow(deprecated)]
     fn wipe_preserves_mode() -> Result<(), Box<dyn std::error::Error>> {
         let mut client = Connection::new_client();
         client.wipe()?;
@@ -2083,6 +2098,7 @@ mod tests {
 
     /// `wipe` preserves the config set on the connection.
     #[test]
+    #[allow(deprecated)]
     fn wipe_preserves_config() -> Result<(), Box<dyn std::error::Error>> {
         use crate::connection::Builder;
 
@@ -2101,6 +2117,7 @@ mod tests {
 
     /// `wipe` clears any application context stored on the connection.
     #[test]
+    #[allow(deprecated)]
     fn wipe_clears_application_context() -> Result<(), Box<dyn std::error::Error>> {
         let mut conn = Connection::new_server();
 
@@ -2116,6 +2133,7 @@ mod tests {
 
     /// A wiped connection can be reused for a subsequent handshake.
     #[test]
+    #[allow(deprecated)]
     fn wipe_allows_connection_reuse() -> Result<(), Box<dyn std::error::Error>> {
         // arbitrary policy. This test has no specific parameter expectations
         let config = build_config(&security::DEFAULT)?;

bindings/rust/extended/s2n-tls/src/connection/builder.rs

@@ -1,13 +1,9 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::{
-    config::Config,
-    connection::Connection,
-    enums::Mode,
-    error::Error,
-    pool::{Pool, PooledConnection},
-};
+#[allow(deprecated)]
+use crate::pool::{Pool, PooledConnection};
+use crate::{config::Config, connection::Connection, enums::Mode, error::Error};
 
 /// A trait indicating that a structure can produce connections.
 pub trait Builder: Clone {
@@ -26,6 +22,7 @@ impl Builder for Config {
 }
 
 /// Produces new connections from a pool of reuseable connections.
+#[allow(deprecated)]
 impl<T: Pool + Clone> Builder for T {
     type Output = PooledConnection<T>;
     fn build_connection(&self, mode: Mode) -> Result<Self::Output, Error> {
@@ -72,6 +69,7 @@ where
 }
 
 #[cfg(test)]
+#[allow(deprecated)]
 mod tests {
     use super::*;
     use crate::pool::ConfigPoolBuilder;

bindings/rust/extended/s2n-tls/src/pool.rs

@@ -1,20 +1,13 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-//! Utilities to handle reusing connections.
-//!
-//! Creating a single new connection requires significant
-//! memory allocations (about 50-60 KB, according to some tests).
-//! Instead of allocating memory for a new connection, existing
-//! memory can be reused by calling
-//! [Connection::wipe()](`crate::connection::Connection::wipe()).
+// This module implements deprecated pool functionality, so internal usage of
+// deprecated items is expected.
+#![allow(deprecated)]
+
+//! Deprecated utilities to handle reusing connections.
 //!
-//! On modern systems with reasonably performant allocators, the benefits of reusing
-//! connections are reduced. Connection reuse is specifically intended for customers
-//! who are sensitive to allocations or for whom allocations are more expensive.
-//! Customers are encouraged to run their own benchmarks to determine the exact
-//! performance benefit. As a starting point, a simple benchmark comparing allocation
-//! against reuse can be found `bench/benches/connection_creation.rs`.
+//! [`Connection::wipe`]
 //!
 //! The [`Pool`] trait allows applications to define an
 //! [Object pool](https://en.wikipedia.org/wiki/Object_pool_pattern) that
@@ -41,6 +34,9 @@ use std::{
 /// When dropped, returns ownership of the connection to
 /// the pool that produced it by calling [`Pool::give`].
 #[derive(Debug)]
+#[deprecated(
+    note = "use `Connection::new()` instead; connection reuse provides negligible performance benefit"
+)]
 pub struct PooledConnection<T: Pool = Arc<dyn Pool>> {
     pool: T,
     conn: Option<Connection>,
@@ -94,6 +90,9 @@ impl<T: Pool + Clone> PooledConnection<T> {
 ///
 /// Minimally, an implementation should call [`Connection::wipe()`]
 /// during [`Self::give`].
+#[deprecated(
+    note = "use `Connection::new()` instead; connection reuse provides negligible performance benefit"
+)]
 pub trait Pool {
     fn mode(&self) -> Mode;
     fn take(&self) -> Result<Connection, Error>;
@@ -131,16 +130,25 @@ impl<T: Pool> Pool for Arc<T> {
 ///
 /// For discussions about expected performance benefits see [self].
 #[derive(Debug)]
+#[deprecated(
+    note = "use `Connection::new()` instead; connection reuse provides negligible performance benefit"
+)]
 pub struct ConfigPool {
     mode: Mode,
     config: Config,
     pool: Mutex<VecDeque<Connection>>,
     max_pool_size: usize,
 }
 
+#[deprecated(
+    note = "use `Connection::new()` instead; connection reuse provides negligible performance benefit"
+)]
 pub type ConfigPoolRef = Arc<ConfigPool>;
 
 /// Builder for [`ConfigPool`].
+#[deprecated(
+    note = "use `Connection::new()` instead; connection reuse provides negligible performance benefit"
+)]
 pub struct ConfigPoolBuilder(ConfigPool);
 impl ConfigPoolBuilder {
     pub fn new(mode: Mode, config: Config) -> Self {

bindings/rust/standard/benchmarks/Cargo.toml

@@ -28,7 +28,3 @@ harness = false
 [[bench]]
 name = "resumption"
 harness = false
-
-[[bench]]
-name = "connection_creation"
-harness = false