From 98e79c2ce54dacabb259b18c90bae06a95068c46 Mon Sep 17 00:00:00 2001 From: James Mayclin Date: Tue, 11 Aug 2026 19:12:03 +0000 Subject: [PATCH 1/2] feat(bindings) deprecate & alias Connection::wipe --- .../extended/s2n-tls-tokio/tests/handshake.rs | 5 ++- .../rust/extended/s2n-tls/src/connection.rs | 30 ++++++++++--- .../s2n-tls/src/connection/builder.rs | 12 +++-- bindings/rust/extended/s2n-tls/src/pool.rs | 19 ++++++++ bindings/rust/standard/benchmarks/Cargo.toml | 4 -- .../benchmarks/benches/connection_creation.rs | 44 ------------------- 6 files changed, 52 insertions(+), 62 deletions(-) delete mode 100644 bindings/rust/standard/benchmarks/benches/connection_creation.rs diff --git a/bindings/rust/extended/s2n-tls-tokio/tests/handshake.rs b/bindings/rust/extended/s2n-tls-tokio/tests/handshake.rs index b550a3e53f7..ea5a74f3b66 100644 --- a/bindings/rust/extended/s2n-tls-tokio/tests/handshake.rs +++ b/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> { } #[tokio::test(flavor = "multi_thread")] +#[allow(deprecated)] async fn handshake_with_pool_multithread() -> Result<(), Box> { const COUNT: usize = 20; const CLIENT_LIMIT: usize = 3; @@ -109,6 +111,7 @@ async fn handshake_with_connection_config() -> Result<(), Box Result<(), Box> { fn with_client_auth(conn: &mut Connection) -> Result<&mut Connection, Error> { conn.set_client_auth_type(ClientAuthType::Optional) diff --git a/bindings/rust/extended/s2n-tls/src/connection.rs b/bindings/rust/extended/s2n-tls/src/connection.rs index e477b4d7bde..d5aecf2bd5e 100644 --- a/bindings/rust/extended/s2n-tls/src/connection.rs +++ b/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(&mut self, wipe: F) -> Result<(), Error> where F: FnOnce(&mut Self) -> Result, @@ -579,15 +580,27 @@ impl Connection { /// /// 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 + /// called. /// /// Corresponds to [`s2n_connection_wipe`]. + #[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> { /// 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> { 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> { 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> { 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> { // arbitrary policy. This test has no specific parameter expectations let config = build_config(&security::DEFAULT)?; diff --git a/bindings/rust/extended/s2n-tls/src/connection/builder.rs b/bindings/rust/extended/s2n-tls/src/connection/builder.rs index 6d637d68fa3..6a761e37605 100644 --- a/bindings/rust/extended/s2n-tls/src/connection/builder.rs +++ b/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 Builder for T { type Output = PooledConnection; fn build_connection(&self, mode: Mode) -> Result { @@ -72,6 +69,7 @@ where } #[cfg(test)] +#[allow(deprecated)] mod tests { use super::*; use crate::pool::ConfigPoolBuilder; diff --git a/bindings/rust/extended/s2n-tls/src/pool.rs b/bindings/rust/extended/s2n-tls/src/pool.rs index 209e1c21438..381945c9e74 100644 --- a/bindings/rust/extended/s2n-tls/src/pool.rs +++ b/bindings/rust/extended/s2n-tls/src/pool.rs @@ -1,6 +1,10 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 +// This module implements deprecated pool functionality, so internal usage of +// deprecated items is expected. +#![allow(deprecated)] + //! Utilities to handle reusing connections. //! //! Creating a single new connection requires significant @@ -41,6 +45,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> { pool: T, conn: Option, @@ -94,6 +101,9 @@ impl PooledConnection { /// /// 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; @@ -131,6 +141,9 @@ impl Pool for Arc { /// /// 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, @@ -138,9 +151,15 @@ pub struct ConfigPool { max_pool_size: usize, } +#[deprecated( + note = "use `Connection::new()` instead; connection reuse provides negligible performance benefit" +)] pub type ConfigPoolRef = Arc; /// 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 { diff --git a/bindings/rust/standard/benchmarks/Cargo.toml b/bindings/rust/standard/benchmarks/Cargo.toml index 6442071af52..f774a8e652b 100644 --- a/bindings/rust/standard/benchmarks/Cargo.toml +++ b/bindings/rust/standard/benchmarks/Cargo.toml @@ -28,7 +28,3 @@ harness = false [[bench]] name = "resumption" harness = false - -[[bench]] -name = "connection_creation" -harness = false diff --git a/bindings/rust/standard/benchmarks/benches/connection_creation.rs b/bindings/rust/standard/benchmarks/benches/connection_creation.rs deleted file mode 100644 index c2a2d78741a..00000000000 --- a/bindings/rust/standard/benchmarks/benches/connection_creation.rs +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -use criterion::{criterion_group, criterion_main, Criterion}; -use s2n_tls::{ - config::Config, - connection::Builder, - enums::Mode, - pool::{ConfigPool, ConfigPoolBuilder, PooledConnection}, -}; -use std::sync::Arc; - -fn connection_wipe(connection_pool: &Arc) { - // get a connection from the pool - let conn = PooledConnection::new(connection_pool).unwrap(); - // "drop" the connection, wiping it and returning it to the pool - drop(conn); -} - -fn connection_new(config: &Config) { - let conn = config - .build_connection(s2n_tls::enums::Mode::Server) - .unwrap(); - drop(conn); -} - -fn connection_creation(c: &mut Criterion) { - let mut group = c.benchmark_group("Connection Creation"); - let config = s2n_tls::config::Builder::new().build().unwrap(); - let connection_pool = ConfigPoolBuilder::new(Mode::Server, config.clone()).build(); - - group.bench_function("connection reuse", |b| { - b.iter(|| connection_wipe(&connection_pool)); - }); - - group.bench_function("connection allocation", |b| { - b.iter(|| connection_new(&config)); - }); - - group.finish(); -} - -criterion_group!(benches, connection_creation); -criterion_main!(benches); From 0ef7a54f352be4ff1fb65083166abcaf1645e836 Mon Sep 17 00:00:00 2001 From: James Mayclin Date: Fri, 21 Aug 2026 00:28:40 +0000 Subject: [PATCH 2/2] address pr feedback --- bindings/rust/extended/s2n-tls/src/connection.rs | 10 +++++----- bindings/rust/extended/s2n-tls/src/pool.rs | 15 ++------------- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/bindings/rust/extended/s2n-tls/src/connection.rs b/bindings/rust/extended/s2n-tls/src/connection.rs index d5aecf2bd5e..df2055899de 100644 --- a/bindings/rust/extended/s2n-tls/src/connection.rs +++ b/bindings/rust/extended/s2n-tls/src/connection.rs @@ -576,13 +576,13 @@ 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. + /// 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" )] diff --git a/bindings/rust/extended/s2n-tls/src/pool.rs b/bindings/rust/extended/s2n-tls/src/pool.rs index 381945c9e74..f90a2b8e3fa 100644 --- a/bindings/rust/extended/s2n-tls/src/pool.rs +++ b/bindings/rust/extended/s2n-tls/src/pool.rs @@ -5,20 +5,9 @@ // deprecated items is expected. #![allow(deprecated)] -//! Utilities to handle reusing connections. +//! Deprecated 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()). -//! -//! 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