diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index 9cd40405c..ccfd29079 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -1953,17 +1953,22 @@ impl SslContextBuilder { } } + /// DEPRECATED: use `Self::replace_ex_data()` to set the data. + /// + /// If this method is called more than once with the same index, any previous + /// value stored in the `SslContextBuilder` may be leaked. This will change in the future release. + /// /// Sets the extra data at the specified index. /// /// This can be used to provide data to callbacks registered with the context. Use the /// `SslContext::new_ex_index` method to create an `Index`. - /// - /// Note that if this method is called multiple times with the same index, any previous - /// value stored in the `SslContextBuilder` will be leaked. #[corresponds(SSL_CTX_set_ex_data)] + #[deprecated( + note = "This may leak memory. Don't rely on leaking. Use `replace_ex_data()` instead." + )] pub fn set_ex_data(&mut self, index: Index, data: T) { unsafe { - self.ctx.set_ex_data(index, data); + self.ctx.write_ex_data(index, data); } } @@ -1974,6 +1979,7 @@ impl SslContextBuilder { /// /// Any previous value will be returned and replaced by the new one. #[corresponds(SSL_CTX_set_ex_data)] + #[doc(alias = "set_ex_data")] pub fn replace_ex_data(&mut self, index: Index, data: T) -> Option { unsafe { self.ctx.replace_ex_data(index, data) } } @@ -2292,10 +2298,11 @@ impl SslContextRef { } } + /// Internal: does not run destructors for the previous value // Unsafe because SSL contexts are not guaranteed to be unique, we call // this only from SslContextBuilder. #[corresponds(SSL_CTX_set_ex_data)] - unsafe fn set_ex_data(&mut self, index: Index, data: T) { + unsafe fn write_ex_data(&mut self, index: Index, data: T) { unsafe { let data = Box::into_raw(Box::new(data)) as *mut c_void; ffi::SSL_CTX_set_ex_data(self.as_ptr(), index.as_raw(), data); @@ -2310,7 +2317,7 @@ impl SslContextRef { return Some(mem::replace(old, data)); } - self.set_ex_data(index, data); + self.write_ex_data(index, data); None } diff --git a/boring/src/ssl/test/mod.rs b/boring/src/ssl/test/mod.rs index 9b7b024c3..98ef1c7cb 100644 --- a/boring/src/ssl/test/mod.rs +++ b/boring/src/ssl/test/mod.rs @@ -1166,3 +1166,47 @@ fn test_ssl_set_compliance() { ssl.set_compliance_policy(CompliancePolicy::NONE) .expect_err("Testing expect err if set compliance policy to NONE"); } + +#[test] +#[allow(deprecated)] +fn ex_data_drop() { + use crate::ssl::SslContextBuilder; + use std::sync::atomic::AtomicU32; + use std::sync::atomic::Ordering::Relaxed; + use std::sync::Arc; + + struct TrackDrop(Arc); + impl Drop for TrackDrop { + fn drop(&mut self) { + self.0.fetch_add(1, Relaxed); + } + } + + let mut ctx = SslContextBuilder::new(SslMethod::tls()).unwrap(); + let index = SslContext::new_ex_index().unwrap(); + let d1 = Arc::new(AtomicU32::new(100)); + let d2 = Arc::new(AtomicU32::new(200)); + let d3 = Arc::new(AtomicU32::new(300)); + ctx.set_ex_data(index, TrackDrop(d1.clone())); + assert_eq!(100, d1.load(Relaxed)); + assert_eq!(200, d2.load(Relaxed)); + ctx.replace_ex_data(index, TrackDrop(d2.clone())); + assert_eq!(101, d1.load(Relaxed)); + assert_eq!(200, d2.load(Relaxed)); + ctx.replace_ex_data(index, TrackDrop(d3.clone())); + assert_eq!(101, d1.load(Relaxed)); + assert_eq!(201, d2.load(Relaxed)); + assert_eq!(300, d3.load(Relaxed)); + drop(ctx); + assert_eq!(101, d1.load(Relaxed)); + assert_eq!(201, d2.load(Relaxed)); + assert_eq!(301, d3.load(Relaxed)); + + let mut ctx2 = SslContextBuilder::new(SslMethod::tls()).unwrap(); + + ctx2.set_ex_data(index, TrackDrop(d1.clone())); + ctx2.set_ex_data(index, TrackDrop(d2.clone())); + drop(ctx2); + assert_eq!(101, d1.load(Relaxed), "set_ex_data has a leak"); + assert_eq!(202, d2.load(Relaxed)); +} diff --git a/boring/src/x509/mod.rs b/boring/src/x509/mod.rs index 8bf96d339..eed30150b 100644 --- a/boring/src/x509/mod.rs +++ b/boring/src/x509/mod.rs @@ -132,6 +132,7 @@ impl X509StoreContextRef { /// This can be used to provide data to callbacks registered with the context. Use the /// `Ssl::new_ex_index` method to create an `Index`. #[corresponds(X509_STORE_CTX_set_ex_data)] + #[doc(alias = "replace_ex_data")] pub fn set_ex_data(&mut self, index: Index, data: T) { if let Some(old) = self.ex_data_mut(index) { *old = data;