From d084114a19a2d576d9a0760e4acc043fa6fa29ec Mon Sep 17 00:00:00 2001 From: Rahul Date: Wed, 26 Jun 2024 11:58:10 -0700 Subject: [PATCH 1/3] initial commit of memory safety test --- crates/openvino/tests/memory-safety.rs | 38 ++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 crates/openvino/tests/memory-safety.rs diff --git a/crates/openvino/tests/memory-safety.rs b/crates/openvino/tests/memory-safety.rs new file mode 100644 index 0000000..03ad365 --- /dev/null +++ b/crates/openvino/tests/memory-safety.rs @@ -0,0 +1,38 @@ +mod fixtures; +mod util; + +use fixtures::mobilenet::Fixture; +use openvino::{Core, DeviceType, ElementType, Shape, Tensor}; +use std::fs; + +#[test] +fn classify_mobilenet() -> anyhow::Result<()> { + let mut core = Core::new()?; + // let model = core.read_model_from_file( + // &Fixture::graph().to_string_lossy(), + // &Fixture::weights().to_string_lossy(), + // )?; + let xml = fs::read_to_string(Fixture::graph())?; + let weights = fs::read(Fixture::weights())?; + + //Construct new tensor with data. + let dims: [i64; 2] = [1, weights.len() as i64]; + let shape = Shape::new(&dims)?; + let mut weights_tensor = Tensor::new(ElementType::U8, &shape)?; + + let buffer = weights_tensor.buffer_mut()?; + for (index, bytes) in weights.iter().enumerate() { + buffer[index] = *bytes; + } + + // let buffer = weights_tensor.buffer_mut()?; + // buffer.copy_from_slice(&weights); + let model = core.read_model_from_buffer(xml.as_bytes(), Some(&weights_tensor))?; + + //std::mem::drop(weights_tensor); + + // Compile the model and infer the results. + //let executable_model = core.compile_model(&model, DeviceType::CPU)?; + //println!("{}",executable_model.get_input()?.get_name()?); + Ok(()) +} From 8096008857ed83a1fe6e309a8fff6c59a9089ddf Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 26 Jun 2024 12:39:41 -0700 Subject: [PATCH 2/3] Clean up, document test --- crates/openvino/tests/memory-safety.rs | 40 ++++++++++++-------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/crates/openvino/tests/memory-safety.rs b/crates/openvino/tests/memory-safety.rs index 03ad365..4ece099 100644 --- a/crates/openvino/tests/memory-safety.rs +++ b/crates/openvino/tests/memory-safety.rs @@ -1,38 +1,36 @@ +//! This test originates from some strange segfaults that we observed while using the OpenVINO C +//! library. Because OpenVINO is taking a pointer to a tensor, we want to be sure that we do the +//! right thing on this side of the FFI boundary. + mod fixtures; -mod util; use fixtures::mobilenet::Fixture; use openvino::{Core, DeviceType, ElementType, Shape, Tensor}; use std::fs; #[test] -fn classify_mobilenet() -> anyhow::Result<()> { +fn memory_safety() -> anyhow::Result<()> { let mut core = Core::new()?; - // let model = core.read_model_from_file( - // &Fixture::graph().to_string_lossy(), - // &Fixture::weights().to_string_lossy(), - // )?; let xml = fs::read_to_string(Fixture::graph())?; let weights = fs::read(Fixture::weights())?; - //Construct new tensor with data. - let dims: [i64; 2] = [1, weights.len() as i64]; - let shape = Shape::new(&dims)?; + // Copy the fixture weights into a tensor. Once we're done here we want to get rid of the + // original weights buffer as a sanity check. + let shape = Shape::new(&[1, weights.len() as i64])?; let mut weights_tensor = Tensor::new(ElementType::U8, &shape)?; + weights_tensor.get_raw_data_mut()?.copy_from_slice(&weights); + drop(weights); - let buffer = weights_tensor.buffer_mut()?; - for (index, bytes) in weights.iter().enumerate() { - buffer[index] = *bytes; - } - - // let buffer = weights_tensor.buffer_mut()?; - // buffer.copy_from_slice(&weights); + // Now create a model from a reference to the weights tensor. We observed SEGFAULT crashes when + // passing weights by value but not by reference. let model = core.read_model_from_buffer(xml.as_bytes(), Some(&weights_tensor))?; + drop(weights_tensor); - //std::mem::drop(weights_tensor); - - // Compile the model and infer the results. - //let executable_model = core.compile_model(&model, DeviceType::CPU)?; - //println!("{}",executable_model.get_input()?.get_name()?); + // Here we double-check that the model is usable. Though it has captured a reference to the + // `weights_tensor` and that tensor has been dropped, whatever OpenVINO is doing internally must + // be safe enough. See + // https://github.com/openvinotoolkit/openvino/blob/d840d86905f013d95cccbafaa0ddff266e250f75/src/inference/src/model_reader.cpp#L178. + assert_eq!(model.get_inputs_len()?, 1); + assert!(core.compile_model(&model, DeviceType::CPU).is_ok()); Ok(()) } From 799c9ef4e01b609733ac57c897bc2f2c7e9c7631 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 26 Jun 2024 12:42:12 -0700 Subject: [PATCH 3/3] Tweak documentation --- crates/openvino/tests/memory-safety.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/openvino/tests/memory-safety.rs b/crates/openvino/tests/memory-safety.rs index 4ece099..66723a0 100644 --- a/crates/openvino/tests/memory-safety.rs +++ b/crates/openvino/tests/memory-safety.rs @@ -1,6 +1,6 @@ //! This test originates from some strange segfaults that we observed while using the OpenVINO C -//! library. Because OpenVINO is taking a pointer to a tensor, we want to be sure that we do the -//! right thing on this side of the FFI boundary. +//! library. Because OpenVINO is taking a pointer to a tensor when constructing a model, we want to +//! be sure that we do the right thing on this side of the FFI boundary. mod fixtures; @@ -21,7 +21,7 @@ fn memory_safety() -> anyhow::Result<()> { weights_tensor.get_raw_data_mut()?.copy_from_slice(&weights); drop(weights); - // Now create a model from a reference to the weights tensor. We observed SEGFAULT crashes when + // Now create a model from a reference to the weights tensor. We observed segfault crashes when // passing weights by value but not by reference. let model = core.read_model_from_buffer(xml.as_bytes(), Some(&weights_tensor))?; drop(weights_tensor);