fix: memory leak in ua::Array::move_into_raw

CHANGELOG.md

@@ -12,6 +12,12 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 - Add back `Server::create_event()` with reordered and additional arguments. This new method
   replaces both `Server::create_event()` and `Server::trigger_event()` that have been removed.
 
+### Fixed
+
+- Fix a memory leak triggered when using `with_*` builder methods on data types for array members
+  (e.g. `ua::RelativePath::with_elements()` would leak the elements that were previously owned by
+  the instance).
+
 ## [0.12.0] - 2026-07-14
 
 ### Changed

Cargo.toml

@@ -69,6 +69,7 @@ rust_2024_compatibility = { level = "warn", priority = -1 }
 tail_expr_drop_order = { level = "allow" }
 trivial_casts = "warn"
 trivial_numeric_casts = "warn"
+unexpected_cfgs = { level = "warn", check-cfg = ['cfg(asan)'] }
 unreachable_pub = "warn"
 # Writing unsafe code is a necessity for FFI wrappers.
 unsafe_code = "allow"

src/lib.rs

@@ -265,6 +265,7 @@ use self::{
     client::ClientContext,
     data_type::{bitmask_ops, data_type, enum_variants},
     monitored_item::create_monitored_items_callback,
+    ua::UaIterExt,
     value::{ArrayValue, NonScalarValue},
 };
 #[cfg(feature = "mbedtls")]

src/server.rs

@@ -30,7 +30,7 @@ use parking_lot::{Condvar, Mutex, MutexGuard};
 
 use crate::{
     Attribute, Attributes, BrowseResult, DEFAULT_PORT_NUMBER, DataType, DataValue, Error, Result,
-    ua,
+    UaIterExt as _, ua,
 };
 
 pub(crate) use self::node_context::NodeContext;
@@ -184,10 +184,10 @@ impl ServerBuilder {
     #[must_use]
     pub fn server_urls(mut self, server_urls: &[&str]) -> Self {
         let config = self.config_mut();
-        let server_urls = server_urls
+        server_urls
             .iter()
-            .map(|server_url| ua::String::new(server_url).unwrap());
-        ua::Array::from_iter(server_urls)
+            .map(|server_url| ua::String::new(server_url).unwrap())
+            .collect_ua_array()
             .move_into_raw(&mut config.serverUrlsSize, &mut config.serverUrls);
         self
     }

src/ua.rs

@@ -73,4 +73,6 @@ pub use self::{
     unit_id::UnitId,
     user_identity_token::UserIdentityToken,
 };
-pub(crate) use self::{client_config::ClientConfig, logger::Logger, server_config::ServerConfig};
+pub(crate) use self::{
+    array::UaIterExt, client_config::ClientConfig, logger::Logger, server_config::ServerConfig,
+};

src/ua/array.rs

@@ -83,22 +83,6 @@ impl<T: DataType> Array<T> {
         Self(State::NonEmpty { ptr: array, size })
     }
 
-    /// Creates new array from existing elements.
-    ///
-    /// This takes ownership of the elements from the given iterator.
-    ///
-    /// # Panics
-    ///
-    /// Enough memory must be available to allocate array.
-    pub(crate) fn from_iter<I: Iterator<Item = T>>(iter: I) -> Self {
-        // This creates a temporary copy by first collecting all elements into a `Vec` and then once
-        // more by copying elements from the `Vec` into the new array from `UA_Array_new()`.
-        //
-        // TODO: Avoid temporary copy. How to deal with unknown size of iterator when initializing a
-        // new array to hold its elements?
-        Self::from_slice(&iter.collect::<Vec<_>>())
-    }
-
     /// Creates new array from existing elements.
     ///
     /// This copies over the elements from the given slice. The array will own the copies, and clean
@@ -401,8 +385,11 @@ impl<T: DataType> Array<T> {
     ///
     /// After this, it is the responsibility of `dst` to eventually clean up the data.
     pub(crate) fn move_into_raw(self, dst_size: &mut usize, dst: &mut *mut T::Inner) {
-        // Make sure to clean up any previous value in target.
-        let _unused = Self::from_raw_parts(*dst_size, *dst);
+        // Make sure to clean up any previous value in target. This must move out of (not copy) the
+        // existing array, otherwise its allocation would be leaked.
+        //
+        // SAFETY: `dst` is the sole owner of the previous array, which we take over and drop here.
+        let _unused = unsafe { Self::move_from_raw_parts(dst_size, dst) };
 
         let (size, ptr) = self.into_raw_parts();
         *dst_size = size;
@@ -456,6 +443,37 @@ impl<T: DataType> Array<T> {
     }
 }
 
+/// Internal-only helper for collection of iterators into [`Array`]s.
+//
+// Might be good to make this public as an impl of [`FromIterator`] once performance is optimized.
+pub(crate) trait UaIterExt<T>
+where
+    T: DataType,
+{
+    /// Takes ownership of the elements from the given iterator.
+    ///
+    /// # Panics
+    ///
+    /// Enough memory must be available to allocate array.
+    fn collect_ua_array(self) -> Array<T>;
+}
+
+impl<I, T> UaIterExt<T> for I
+where
+    I: Iterator<Item = T>,
+    T: DataType,
+{
+    fn collect_ua_array(self) -> Array<T> {
+        // This creates a temporary copy by first collecting all elements into a `Vec` and then once
+        // more by copying elements from the `Vec` into the new array from `UA_Array_new()`.
+        //
+        // TODO: Avoid temporary copy. How to deal with unknown size of iterator when initializing a
+        // new array to hold its elements? (possible optimization opportunity with a specialized
+        // implementation for `ExactSizeIterator`)
+        Array::from_slice(&self.collect::<Vec<_>>())
+    }
+}
+
 // SAFETY: Arrays in `open62541` can be sent across thread boundaries: they may be dropped in other
 // threads as long as their items are `Send`.
 unsafe impl<T: DataType + Send> Send for Array<T> {}
@@ -679,4 +697,16 @@ mod tests {
         .join()
         .expect("join thread");
     }
+
+    // Simple probe to verify that the behavior of `Array::move_into_raw()` doesn't leak.
+    // Only valuable when run with `-Zsanitizer=address`, so it's being gated behind an explicit cfg.
+    #[cfg(asan)]
+    #[test]
+    fn move_into_raw_leak_probe() {
+        let array_src = ua::Array::<ua::UInt32>::new(42);
+        let (mut size, mut ptr) = ua::Array::<ua::UInt32>::new(24).into_raw_parts();
+        // This would fail under ASAN with previous impl, now succeeds.
+        array_src.move_into_raw(&mut size, &mut ptr);
+        unsafe { ua::Array::<ua::UInt32>::move_from_raw_parts(&mut size, &mut ptr) }.unwrap();
+    }
 }

src/ua/data_types/application_description.rs

@@ -1,4 +1,4 @@
-use crate::{DataType as _, ua};
+use crate::{DataType as _, UaIterExt as _, ua};
 
 crate::data_type!(ApplicationDescription);
 
@@ -86,10 +86,10 @@ impl ApplicationDescription {
     /// The strings must not contain any NUL bytes.
     #[must_use]
     pub fn with_discovery_urls(mut self, discovery_urls: &[&str]) -> Self {
-        let discovery_urls = discovery_urls
+        discovery_urls
             .iter()
-            .map(|discovery_url| ua::String::new(discovery_url).unwrap());
-        ua::Array::from_iter(discovery_urls)
+            .map(|discovery_url| ua::String::new(discovery_url).unwrap())
+            .collect_ua_array()
             .move_into_raw(&mut self.0.discoveryUrlsSize, &mut self.0.discoveryUrls);
         self
     }

src/ua/data_types/browse_next_request.rs

@@ -1,4 +1,4 @@
-use crate::{DataType as _, ServiceRequest, ua};
+use crate::{DataType as _, ServiceRequest, UaIterExt as _, ua};
 
 crate::data_type!(BrowseNextRequest);
 
@@ -8,15 +8,14 @@ impl BrowseNextRequest {
         mut self,
         continuation_points: &[ua::ContinuationPoint],
     ) -> Self {
-        let array = ua::Array::from_iter(
-            continuation_points
-                .iter()
-                .map(ua::ContinuationPoint::to_byte_string),
-        );
-        array.move_into_raw(
-            &mut self.0.continuationPointsSize,
-            &mut self.0.continuationPoints,
-        );
+        continuation_points
+            .iter()
+            .map(ua::ContinuationPoint::to_byte_string)
+            .collect_ua_array()
+            .move_into_raw(
+                &mut self.0.continuationPointsSize,
+                &mut self.0.continuationPoints,
+            );
         self
     }
 

src/ua/data_types/content_filter_element.rs

@@ -1,4 +1,4 @@
-use crate::{DataType, FilterOperand, ua};
+use crate::{DataType, FilterOperand, UaIterExt as _, ua};
 
 crate::data_type!(ContentFilterElement);
 
@@ -11,12 +11,11 @@ impl ContentFilterElement {
 
     #[must_use]
     pub fn with_filter_operands(mut self, filter_operands: &[impl FilterOperand]) -> Self {
-        let array = ua::Array::from_iter(
-            filter_operands
-                .iter()
-                .map(FilterOperand::to_extension_object),
-        );
-        array.move_into_raw(&mut self.0.filterOperandsSize, &mut self.0.filterOperands);
+        filter_operands
+            .iter()
+            .map(FilterOperand::to_extension_object)
+            .collect_ua_array()
+            .move_into_raw(&mut self.0.filterOperandsSize, &mut self.0.filterOperands);
         self
     }
 }

src/ua/data_types/delete_monitored_items_request.rs

@@ -1,4 +1,4 @@
-use crate::ua;
+use crate::{UaIterExt as _, ua};
 
 crate::data_type!(DeleteMonitoredItemsRequest);
 
@@ -11,15 +11,14 @@ impl DeleteMonitoredItemsRequest {
 
     #[must_use]
     pub fn with_monitored_item_ids(mut self, monitored_item_ids: &[ua::MonitoredItemId]) -> Self {
-        let array = ua::Array::from_iter(
-            monitored_item_ids
-                .iter()
-                .map(|monitored_item_id| monitored_item_id.as_id().to_uint32()),
-        );
-        array.move_into_raw(
-            &mut self.0.monitoredItemIdsSize,
-            &mut self.0.monitoredItemIds,
-        );
+        monitored_item_ids
+            .iter()
+            .map(|monitored_item_id| monitored_item_id.as_id().to_uint32())
+            .collect_ua_array()
+            .move_into_raw(
+                &mut self.0.monitoredItemIdsSize,
+                &mut self.0.monitoredItemIds,
+            );
         self
     }
 }

src/ua/data_types/delete_subscriptions_request.rs

@@ -1,16 +1,15 @@
-use crate::ua;
+use crate::{UaIterExt as _, ua};
 
 crate::data_type!(DeleteSubscriptionsRequest);
 
 impl DeleteSubscriptionsRequest {
     #[must_use]
     pub fn with_subscription_ids(mut self, subscription_ids: &[ua::SubscriptionId]) -> Self {
-        let array = ua::Array::from_iter(
-            subscription_ids
-                .iter()
-                .map(|subscription_id| subscription_id.as_id().to_uint32()),
-        );
-        array.move_into_raw(&mut self.0.subscriptionIdsSize, &mut self.0.subscriptionIds);
+        subscription_ids
+            .iter()
+            .map(|subscription_id| subscription_id.as_id().to_uint32())
+            .collect_ua_array()
+            .move_into_raw(&mut self.0.subscriptionIdsSize, &mut self.0.subscriptionIds);
         self
     }
 }

src/ua/data_types/node_attributes/variable_attributes.rs

@@ -1,4 +1,4 @@
-use crate::{DataType as _, ua};
+use crate::{DataType as _, UaIterExt as _, ua};
 
 impl super::VariableAttributes {
     #[must_use]
@@ -17,9 +17,11 @@ impl super::VariableAttributes {
 
     #[must_use]
     pub fn with_array_dimensions(mut self, array_dimensions: &[u32]) -> Self {
-        let array_dimensions =
-            ua::Array::from_iter(array_dimensions.iter().copied().map(ua::UInt32::new));
         array_dimensions
+            .iter()
+            .copied()
+            .map(ua::UInt32::new)
+            .collect_ua_array()
             .move_into_raw(&mut self.0.arrayDimensionsSize, &mut self.0.arrayDimensions);
         self.0.specifiedAttributes |= ua::SpecifiedAttributes::ARRAYDIMENSIONS.as_u32();
         self