From 668fb44b15b8e2ce0a6c13681a2bd6d2d62d3dab Mon Sep 17 00:00:00 2001 From: Ohad Ravid Date: Fri, 15 Jul 2022 10:36:09 +0300 Subject: [PATCH 1/3] Make COMLibrary !Send, remove CoUninitialize calls --- src/connection.rs | 33 ++++++++++++++++++++------------- src/tests.rs | 6 ++---- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/connection.rs b/src/connection.rs index 876e79e..bb67cc6 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -1,6 +1,7 @@ use crate::utils::{check_hres, WMIError}; use crate::BStr; use log::debug; +use std::marker::PhantomData; use std::ptr; use std::ptr::NonNull; use std::rc::Rc; @@ -14,21 +15,23 @@ use winapi::{ wtypesbase::CLSCTX_INPROC_SERVER, }, um::{ - combaseapi::{ - CoCreateInstance, CoInitializeEx, CoInitializeSecurity, CoSetProxyBlanket, - CoUninitialize, - }, + combaseapi::{CoCreateInstance, CoInitializeEx, CoInitializeSecurity, CoSetProxyBlanket}, objbase::COINIT_MULTITHREADED, objidl::EOAC_NONE, wbemcli::{CLSID_WbemLocator, IID_IWbemLocator, IWbemLocator, IWbemServices}, }, }; -pub struct COMLibrary {} +/// A Marker trait to indicate that the current thread was `CoInitialize`d. +pub struct COMLibrary { + // Force the type to be `!Send`, as each thread must be initialized separately. + _phantom: PhantomData<*mut ()>, +} /// Initialize COM. /// -/// COM will be `CoUninitialize`d after this object is dropped. +/// `CoUninitialize` will NOT be called when dropped. +/// See: https://github.com/microsoft/windows-rs/issues/1169#issuecomment-926877227 /// impl COMLibrary { /// `CoInitialize`s the COM library for use by the calling thread. @@ -36,7 +39,9 @@ impl COMLibrary { pub fn new() -> Result { unsafe { check_hres(CoInitializeEx(ptr::null_mut(), COINIT_MULTITHREADED))? } - let instance = Self {}; + let instance = Self { + _phantom: PhantomData, + }; instance.init_security()?; @@ -48,7 +53,9 @@ impl COMLibrary { pub fn without_security() -> Result { unsafe { check_hres(CoInitializeEx(ptr::null_mut(), COINIT_MULTITHREADED))? } - let instance = Self {}; + let instance = Self { + _phantom: PhantomData, + }; Ok(instance) } @@ -72,11 +79,11 @@ impl COMLibrary { } } -impl Drop for COMLibrary { - fn drop(&mut self) { - unsafe { CoUninitialize() }; - } -} +/// ```compile_fail +/// let com = COMLibrary::new().unwrap(); +/// _test_com_lib_not_send(com); +/// ``` +fn _test_com_lib_not_send(_s: impl Send) {} pub struct WMIConnection { _com_con: Option>, diff --git a/src/tests.rs b/src/tests.rs index e710368..d69f174 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -3,13 +3,11 @@ use crate::WMIConnection; pub mod fixtures { use super::*; - use lazy_static::lazy_static; - // This way we only setup COM security once during tests. // We can't use `std::sync::Once` because we have to keep the `COM_LIB` object alive for the // entire run. - lazy_static! { - static ref COM_LIB: COMLibrary = COMLibrary::new().unwrap(); + thread_local! { + static COM_LIB: COMLibrary = COMLibrary::new().unwrap(); } pub fn wmi_con() -> WMIConnection { From c0d3dfc86008ba3b1742ff54fc7902e775d8a5c6 Mon Sep 17 00:00:00 2001 From: Ohad Ravid Date: Fri, 15 Jul 2022 10:57:51 +0300 Subject: [PATCH 2/3] Make COMLibrary Copy, move `assume_initialized` to COMLib from WMICon --- src/connection.rs | 74 +++++++++++++++++++++-------------------------- src/tests.rs | 10 +++---- 2 files changed, 38 insertions(+), 46 deletions(-) diff --git a/src/connection.rs b/src/connection.rs index bb67cc6..9391258 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -4,7 +4,6 @@ use log::debug; use std::marker::PhantomData; use std::ptr; use std::ptr::NonNull; -use std::rc::Rc; use winapi::{ shared::{ ntdef::NULL, @@ -22,7 +21,9 @@ use winapi::{ }, }; -/// A Marker trait to indicate that the current thread was `CoInitialize`d. +/// A marker to indicate that the current thread was `CoInitialize`d. +/// It can be freely copied within the same thread. +#[derive(Clone, Copy)] pub struct COMLibrary { // Force the type to be `!Send`, as each thread must be initialized separately. _phantom: PhantomData<*mut ()>, @@ -37,12 +38,7 @@ impl COMLibrary { /// `CoInitialize`s the COM library for use by the calling thread. /// pub fn new() -> Result { - unsafe { check_hres(CoInitializeEx(ptr::null_mut(), COINIT_MULTITHREADED))? } - - let instance = Self { - _phantom: PhantomData, - }; - + let instance = Self::without_security()?; instance.init_security()?; Ok(instance) @@ -60,6 +56,31 @@ impl COMLibrary { Ok(instance) } + /// Assumes that COM was already initialized for this thread. + /// + /// # Safety + /// + /// This function is unsafe as it is the caller's responsibility to ensure that COM is initialized + /// and will not be uninitialized while any instance of object is in scope. + /// + /// ```edition2018 + /// # fn main() -> Result<(), wmi::WMIError> { + /// # use wmi::*; + /// # use serde::Deserialize; + /// # let _actual_com = COMLibrary::new()?; + /// let initialized_com = unsafe { COMLibrary::assume_initialized() }; + /// + /// // Later, in the same thread. + /// let wmi_con = WMIConnection::with_namespace_path("ROOT\\CIMV2", initialized_com)?; + /// # Ok(()) + /// # } + /// ``` + pub unsafe fn assume_initialized() -> Self { + Self { + _phantom: PhantomData, + } + } + fn init_security(&self) -> Result<(), WMIError> { unsafe { check_hres(CoInitializeSecurity( @@ -86,7 +107,7 @@ impl COMLibrary { fn _test_com_lib_not_send(_s: impl Send) {} pub struct WMIConnection { - _com_con: Option>, + _com_con: COMLibrary, p_loc: Option>, p_svc: Option>, } @@ -107,7 +128,7 @@ impl WMIConnection { } /// Creates a connection with a default `CIMV2` namespace path. - pub fn new(com_lib: Rc) -> Result { + pub fn new(com_lib: COMLibrary) -> Result { Self::with_namespace_path("ROOT\\CIMV2", com_lib) } @@ -123,10 +144,10 @@ impl WMIConnection { /// ``` pub fn with_namespace_path( namespace_path: &str, - com_lib: Rc, + com_lib: COMLibrary, ) -> Result { let mut instance = Self { - _com_con: Some(com_lib), + _com_con: com_lib, p_loc: None, p_svc: None, }; @@ -136,35 +157,6 @@ impl WMIConnection { Ok(instance) } - /// Like `with_namespace_path`, but assumes that COM is managed externally. - /// - /// # Safety - /// - /// This function is unsafe as it is the caller's responsibility to ensure that COM is initialized and will not be uninitialized before the connection object is dropped. - /// - /// ```edition2018 - /// # fn main() -> Result<(), wmi::WMIError> { - /// # use wmi::*; - /// # use serde::Deserialize; - /// let _initialized_com = COMLibrary::new()?; - /// - /// // Later, in the same thread. - /// let wmi_con = unsafe { WMIConnection::with_initialized_com(Some("ROOT\\CIMV2"))? }; - /// # Ok(()) - /// # } - /// ``` - pub unsafe fn with_initialized_com(namespace_path: Option<&str>) -> Result { - let mut instance = Self { - _com_con: None, - p_loc: None, - p_svc: None, - }; - - instance.create_and_set_proxy(namespace_path)?; - - Ok(instance) - } - pub fn svc(&self) -> *mut IWbemServices { self.p_svc.unwrap().as_ptr() } diff --git a/src/tests.rs b/src/tests.rs index d69f174..b0b547a 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -3,15 +3,15 @@ use crate::WMIConnection; pub mod fixtures { use super::*; - // This way we only setup COM security once during tests. - // We can't use `std::sync::Once` because we have to keep the `COM_LIB` object alive for the - // entire run. + // This way we only setup COM security once per thread during tests. thread_local! { - static COM_LIB: COMLibrary = COMLibrary::new().unwrap(); + static COM_LIB: COMLibrary = COMLibrary::without_security().unwrap(); } pub fn wmi_con() -> WMIConnection { - let wmi_con = WMIConnection::new(COMLibrary::without_security().unwrap().into()).unwrap(); + let com_lib = COM_LIB.with(|com| *com); + + let wmi_con = WMIConnection::new(com_lib).unwrap(); wmi_con } From 57abb0df2dd8fadfc06764845f70367c4810d76c Mon Sep 17 00:00:00 2001 From: Ohad Ravid Date: Fri, 15 Jul 2022 10:58:57 +0300 Subject: [PATCH 3/3] Bump version --- Cargo.toml | 2 +- README.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index eb488cf..5879844 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "wmi" -version = "0.10.0" +version = "0.11.0" authors = ["Ohad Ravid "] edition = "2018" license = "MIT OR Apache-2.0" diff --git a/README.md b/README.md index 84f6b04..a0d8db1 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ WMI (Windows Management Instrumentation) crate for rust. ```toml # Cargo.toml [dependencies] -wmi = "0.10" +wmi = "0.11" ``` @@ -60,7 +60,7 @@ If you prefer to use the `time` crate instead of the default `chrono`, include ` ```toml [dependencies] -wmi-rs = { version = "0.10", default-features = false, features = ["time"] } +wmi-rs = { version = "0.11", default-features = false, features = ["time"] } ``` and use the `WMIOffsetDateTime` wrapper instead of the the `WMIDateTime` wrapper.