Merge pull request #63 from daladim/safer_traces_part2

 Safer traces, part 2: Trace builder

examples/dns.rs

@@ -9,7 +9,6 @@ use ferrisetw::schema_locator::SchemaLocator;
 use ferrisetw::native::etw_types::EventRecord;
 use ferrisetw::trace::UserTrace;
 use ferrisetw::parser::TryParse;
-use ferrisetw::trace::TraceBaseTrait;
 use ferrisetw::schema::Schema;
 
 
@@ -74,16 +73,16 @@ fn main() {
         .trace_flags(TraceFlags::EVENT_ENABLE_PROPERTY_PROCESS_START_KEY)
         .build();
 
-    let mut trace = UserTrace::new()
+    let trace = UserTrace::new()
         .enable(dns_provider)
-        .start()
+        .start_and_process()
         .unwrap();
 
     println!("ID   Status Options         Ty Name       Results");
 
-    std::thread::sleep(Duration::new(120, 0));
-    trace.stop();
+    std::thread::sleep(Duration::new(20, 0));
 
+    trace.stop().unwrap(); // This is not required, as it will automatically be stopped on Drop
     println!("Done: {:?} events", N_EVENTS);
 }
 

examples/kernel_trace.rs

@@ -31,12 +31,12 @@ fn main() {
         .add_callback(image_load_callback)
         .build();
 
-    let mut trace = KernelTrace::new()
+    let kernel_trace = KernelTrace::new()
         .named(String::from("MyKernelProvider"))
         .enable(provider)
-        .start()
+        .start_and_process()
         .unwrap();
 
     std::thread::sleep(Duration::new(20, 0));
-    trace.stop();
+    kernel_trace.stop().unwrap(); // This is not required, as it will automatically be stopped on Drop
 }

examples/multiple_providers.rs

@@ -60,12 +60,13 @@ fn main() {
         .add_callback(registry_callback)
         .build();
 
-    let mut trace = UserTrace::new()
+    let user_trace = UserTrace::new()
         .enable(process_provider)
         .enable(tcpip_provider)
-        .start()
+        .start_and_process()
         .unwrap();
 
     std::thread::sleep(Duration::new(10, 0));
-    trace.stop();
+
+    user_trace.stop().unwrap(); // optional. Simply dropping user_trace has the same effect
 }

examples/user_trace.rs

@@ -33,12 +33,22 @@ fn main() {
         .add_callback(process_callback)
         .build();
 
-    let mut trace = UserTrace::new()
+    let (_user_trace, handle) = UserTrace::new()
         .named(String::from("MyProvider"))
         .enable(process_provider)
         .start()
         .unwrap();
 
+    // This example uses `process_from_handle` rather than the more convient `start_and_process`, because why not.
+    std::thread::spawn(move || {
+        let status = UserTrace::process_from_handle(handle);
+        // This code will be executed when the trace stops. Examples:
+        // * when it is dropped
+        // * when it is manually stopped (either by user_trace.stop, or by the `logman stop -ets MyProvider` command)
+        println!("Trace ended with status {:?}", status);
+    });
+
     std::thread::sleep(Duration::new(20, 0));
-    trace.stop();
+
+    // user_trace will be dropped (and stopped) here
 }

src/lib.rs

@@ -50,7 +50,7 @@
 //! use ferrisetw::parser::Parser;
 //! use ferrisetw::parser::TryParse;
 //! use ferrisetw::provider::Provider;
-//! use ferrisetw::trace::{UserTrace, TraceTrait, TraceBaseTrait};
+//! use ferrisetw::trace::{UserTrace, TraceTrait};
 //!
 //! fn process_callback(record: &EventRecord, schema_locator: &SchemaLocator) {
 //!     // Within the callback we first locate the proper Schema for the event
@@ -84,12 +84,14 @@
 //!         .build();
 //!
 //!     // We start a trace session for the previously registered provider
-//!     // This call will spawn a new thread which listens to the events
+//!     // Callbacks will be run in a separate thread.
 //!     let mut trace = UserTrace::new()
 //!         .named(String::from("MyProvider"))
 //!         .enable(process_provider)
-//!         // .enable(other_provider) // it is possible to enable multiple providers on the same trace
-//!         .start()
+//!         // .enable(other_provider) // It is possible to enable multiple providers on the same trace.
+//!         .start_and_process()       // This call will spawn the thread for you.
+//!                                    // See the doc for alternative ways of processing the trace,
+//!                                    // with more or less flexibility regarding this spawned thread.
 //!         .unwrap();
 //!
 //!     std::thread::sleep(std::time::Duration::from_secs(3));

src/native/etw_types.rs

@@ -7,28 +7,26 @@
 //! In most cases a user of the crate won't have to deal with this and can directly obtain the data
 //! needed by using the functions exposed by the modules at the crate level
 use crate::provider::event_filter::EventFilterDescriptor;
-use crate::provider::{Provider, TraceFlags};
-use crate::trace::{TraceData, TraceProperties, TraceTrait};
+use crate::provider::TraceFlags;
+use crate::trace::{CallbackData, TraceProperties, TraceTrait};
 use std::ffi::c_void;
 use std::fmt::Formatter;
 use std::marker::PhantomData;
+use std::sync::Arc;
+
 use windows::core::GUID;
-use windows::core::PSTR;
-use windows::Win32::Foundation::MAX_PATH;
+use windows::core::PWSTR;
 use windows::Win32::System::Diagnostics::Etw;
 use windows::Win32::System::Diagnostics::Etw::EVENT_FILTER_DESCRIPTOR;
+use widestring::{U16CStr, U16CString};
 
 mod event_record;
 pub use event_record::EventRecord;
 
 mod extended_data;
 pub use extended_data::{ExtendedDataItem, EventHeaderExtendedDataItem};
 
-// typedef ULONG64 TRACEHANDLE, *PTRACEHANDLE;
-pub(crate) type TraceHandle = u64;
-pub(crate) type EvenTraceControl = Etw::EVENT_TRACE_CONTROL;
-
-pub const INVALID_TRACE_HANDLE: TraceHandle = u64::MAX;
+pub const TRACE_NAME_MAX_CHARS: usize = 200; // Microsoft documentation says the limit is 1024, but do not trust us. Experience shows that traces with names longer than ~240 character silently fail.
 
 /// This enum is <https://learn.microsoft.com/en-us/windows/win32/api/evntrace/ne-evntrace-trace_query_info_class>
 ///
@@ -134,139 +132,148 @@ impl From<ProcessTraceMode> for u32 {
     }
 }
 
-/// Newtype wrapper over an [EVENT_TRACE_PROPERTIES]
+
+/// Wrapper over an [EVENT_TRACE_PROPERTIES](https://docs.microsoft.com/en-us/windows/win32/api/evntrace/ns-evntrace-event_trace_properties), and its allocated companion members
 ///
-/// [EVENT_TRACE_PROPERTIES]: https://microsoft.github.io/windows-docs-rs/doc/bindings/Windows/Win32/Etw/struct.EVENT_TRACE_PROPERTIES.html
+/// The [EventTraceProperties] struct contains the information about a tracing session, this struct
+/// also needs two buffers right after it to hold the log file name and the session name. This struct
+/// provides the full definition of the properties plus the the allocation for both names
 #[repr(C)]
 #[derive(Clone, Copy)]
-pub struct EventTraceProperties(Etw::EVENT_TRACE_PROPERTIES);
-
-impl std::fmt::Debug for EventTraceProperties {
-    fn fmt(&self, _f: &mut Formatter<'_>) -> std::fmt::Result {
-        todo!()
-    }
+pub struct EventTraceProperties {
+    etw_trace_properties: Etw::EVENT_TRACE_PROPERTIES,
+    wide_trace_name: [u16; TRACE_NAME_MAX_CHARS+1],    // The +1 leaves space for the final null widechar.
+    wide_log_file_name: [u16; TRACE_NAME_MAX_CHARS+1], // The +1 leaves space for the final null widechar. Not used currently, but this may be useful when resolving https://github.com/n4r1b/ferrisetw/issues/7
 }
 
-impl Default for EventTraceProperties {
-    fn default() -> Self {
-        unsafe { std::mem::zeroed::<EventTraceProperties>() }
-    }
-}
-
-impl std::ops::Deref for EventTraceProperties {
-    type Target = Etw::EVENT_TRACE_PROPERTIES;
-
-    fn deref(&self) -> &self::Etw::EVENT_TRACE_PROPERTIES {
-        &self.0
-    }
-}
 
-impl std::ops::DerefMut for EventTraceProperties {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.0
+impl std::fmt::Debug for EventTraceProperties {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let name = U16CString::from_vec_truncate(self.wide_trace_name).to_string_lossy();
+        f.debug_struct("EventTraceProperties")
+            .field("name", &name)
+            .finish()
     }
 }
 
-/// Complete Trace Properties struct
-///
-/// The [EventTraceProperties] struct contains the information about a tracing session, this struct
-/// also needs two buffers right after it to hold the log file name and the session name. This struct
-/// provides the full definition of the properties plus the the allocation for both names
-///
-/// See: [EVENT_TRACE_PROPERTIES](https://docs.microsoft.com/en-us/windows/win32/api/evntrace/ns-evntrace-event_trace_properties)
-#[repr(C)]
-#[derive(Clone, Copy, Debug)]
-pub struct TraceInfo {
-    pub properties: EventTraceProperties,
-    trace_name: [u8; MAX_PATH as usize],
-    log_file_name: [u8; MAX_PATH as usize],
-}
-
-impl TraceInfo {
-    pub(crate) fn fill<T>(
-        &mut self,
-        trace_name: &str,
+impl EventTraceProperties {
+    /// Create a new instance
+    ///
+    /// # Notes
+    /// `trace_name` is limited to 200 characters.
+    pub(crate) fn new<T>(
+        trace_name: &U16CStr,
         trace_properties: &TraceProperties,
-        providers: &[Provider],
-    ) where
-        T: TraceTrait,
+        enable_flags: Etw::EVENT_TRACE_FLAG,
+    ) -> Self
+    where
+        T: TraceTrait
     {
-        self.properties.0.Wnode.BufferSize = std::mem::size_of::<TraceInfo>() as u32;
-        self.properties.0.Wnode.Guid = T::trace_guid();
-        self.properties.0.Wnode.Flags = Etw::WNODE_FLAG_TRACED_GUID;
-        self.properties.0.Wnode.ClientContext = 1; // QPC clock resolution
-        self.properties.0.BufferSize = trace_properties.buffer_size;
-        self.properties.0.MinimumBuffers = trace_properties.min_buffer;
-        self.properties.0.MaximumBuffers = trace_properties.max_buffer;
-        self.properties.0.FlushTimer = trace_properties.flush_timer;
+        let mut etw_trace_properties = Etw::EVENT_TRACE_PROPERTIES::default();
+
+        etw_trace_properties.Wnode.BufferSize = std::mem::size_of::<EventTraceProperties>() as u32;
+        etw_trace_properties.Wnode.Guid = T::trace_guid();
+        etw_trace_properties.Wnode.Flags = Etw::WNODE_FLAG_TRACED_GUID;
+        etw_trace_properties.Wnode.ClientContext = 1; // QPC clock resolution
+        etw_trace_properties.BufferSize = trace_properties.buffer_size;
+        etw_trace_properties.MinimumBuffers = trace_properties.min_buffer;
+        etw_trace_properties.MaximumBuffers = trace_properties.max_buffer;
+        etw_trace_properties.FlushTimer = trace_properties.flush_timer;
 
         if trace_properties.log_file_mode != 0 {
-            self.properties.0.LogFileMode = trace_properties.log_file_mode;
+            etw_trace_properties.LogFileMode = trace_properties.log_file_mode;
         } else {
-            self.properties.0.LogFileMode =
+            etw_trace_properties.LogFileMode =
                 u32::from(LoggingMode::RealTime) | u32::from(LoggingMode::NoPerProcBuffering);
         }
 
-        self.properties.0.LogFileMode |= T::augmented_file_mode();
-        self.properties.0.EnableFlags = Etw::EVENT_TRACE_FLAG(T::enable_flags(providers));
+        etw_trace_properties.LogFileMode |= T::augmented_file_mode();
+        etw_trace_properties.EnableFlags = enable_flags;
+
+        // etw_trace_properties.LogFileNameOffset must be 0, but this will change when https://github.com/n4r1b/ferrisetw/issues/7 is resolved
+        // > If you do not want to log events to a log file (for example, if you specify EVENT_TRACE_REAL_TIME_MODE only), set LogFileNameOffset to 0.
+        // (https://learn.microsoft.com/en-us/windows/win32/api/evntrace/ns-evntrace-event_trace_properties)
+        etw_trace_properties.LoggerNameOffset = offset_of!(EventTraceProperties, wide_log_file_name) as u32;
+
+        // https://learn.microsoft.com/en-us/windows/win32/api/evntrace/ns-evntrace-event_trace_properties#remarks
+        // > You do not copy the session name to the offset. The StartTrace function copies the name for you.
+        //
+        // Let's do it anyway, even though that's not required
+        let mut s = Self {
+            etw_trace_properties,
+            wide_trace_name: [0u16; TRACE_NAME_MAX_CHARS+1],
+            wide_log_file_name: [0u16; TRACE_NAME_MAX_CHARS+1],
+        };
+        let name_len = trace_name.len().min(TRACE_NAME_MAX_CHARS);
+        s.wide_trace_name[..name_len].copy_from_slice(&trace_name.as_slice()[..name_len]);
+
+        s
+    }
 
-        self.properties.0.LoggerNameOffset = offset_of!(TraceInfo, log_file_name) as u32;
-        self.trace_name[..trace_name.len()].copy_from_slice(trace_name.as_bytes())
+    /// Gets a pointer to the wrapped [Etw::EVENT_TRACE_PROPERTIES]
+    ///
+    /// # Safety
+    ///
+    /// The API enforces this points to an allocated, valid `EVENT_TRACE_PROPERTIES` instance.
+    /// As evey other mutable raw pointer, you should not use it in case someone else is keeping a reference to this object.
+    ///
+    /// Note that `OpenTraceA` **will** modify its content on output.
+    pub unsafe fn as_mut_ptr(&mut self) -> *mut Etw::EVENT_TRACE_PROPERTIES {
+        &mut self.etw_trace_properties as *mut Etw::EVENT_TRACE_PROPERTIES
     }
-}
 
-impl Default for TraceInfo {
-    fn default() -> Self {
-        let properties = EventTraceProperties::default();
-        TraceInfo {
-            properties,
-            trace_name: [0; 260],
-            log_file_name: [0; 260],
-        }
+    pub fn trace_name_array(&self) -> &[u16] {
+        &self.wide_trace_name
     }
 }
 
-/// Newtype wrapper over an [EVENT_TRACE_LOGFILEA]
+/// Newtype wrapper over an [EVENT_TRACE_LOGFILEW]
 ///
-/// [EVENT_TRACE_LOGFILEA]: https://microsoft.github.io/windows-docs-rs/doc/bindings/Windows/Win32/Etw/struct.EVENT_TRACE_LOGFILEA.html
+/// Its lifetime is tied a to [`CallbackData`] because it contains raw pointers to it.
+///
+/// [EVENT_TRACE_LOGFILEW]: https://microsoft.github.io/windows-docs-rs/doc/windows/Win32/System/Diagnostics/Etw/struct.EVENT_TRACE_LOGFILEW.html
 #[repr(C)]
-#[derive(Clone, Copy)]
-pub struct EventTraceLogfile<'tracedata> {
-    native: Etw::EVENT_TRACE_LOGFILEA,
-    lifetime: PhantomData<&'tracedata TraceData>,
+#[derive(Clone)]
+pub struct EventTraceLogfile<'callbackdata> {
+    native: Etw::EVENT_TRACE_LOGFILEW,
+    wide_logger_name: U16CString,
+    lifetime: PhantomData<&'callbackdata CallbackData>,
 }
 
-impl<'tracedata> EventTraceLogfile<'tracedata> {
+impl<'callbackdata> EventTraceLogfile<'callbackdata> {
     /// Create a new instance
-    pub fn create(trace_data: &'tracedata Box<TraceData>, callback: unsafe extern "system" fn(*mut Etw::EVENT_RECORD)) -> Self {
-        let mut log_file = EventTraceLogfile::default();
+    pub fn create(callback_data: &'callbackdata Box<Arc<CallbackData>>, mut wide_logger_name: U16CString, callback: unsafe extern "system" fn(*mut Etw::EVENT_RECORD)) -> Self {
+        let mut native = Etw::EVENT_TRACE_LOGFILEW::default();
 
-        let not_really_mut_ptr = trace_data.name.as_ptr() as *mut _; // That's kind-of fine because the logger name is _not supposed_ to be changed by Windows APIs
-        log_file.native.LoggerName = PSTR(not_really_mut_ptr);
-        log_file.native.Anonymous1.ProcessTraceMode =
+        native.LoggerName = PWSTR(wide_logger_name.as_mut_ptr());
+        native.Anonymous1.ProcessTraceMode =
             u32::from(ProcessTraceMode::RealTime) | u32::from(ProcessTraceMode::EventRecord);
 
-        log_file.native.Anonymous2.EventRecordCallback = Some(callback);
+        native.Anonymous2.EventRecordCallback = Some(callback);
 
-        let not_really_mut_ptr = trace_data.as_ref() as *const TraceData as *const c_void as *mut c_void; // That's kind-of fine because the user context is _not supposed_ to be changed by Windows APIs
-        log_file.native.Context = not_really_mut_ptr;
+        let not_really_mut_ptr = callback_data.as_ref() as *const Arc<CallbackData> as *const c_void as *mut c_void; // That's kind-of fine because the user context is _not supposed_ to be changed by Windows APIs
+        native.Context = not_really_mut_ptr;
 
-        log_file
+        Self {
+            native,
+            wide_logger_name,
+            lifetime: PhantomData,
+        }
     }
 
     /// Retrieve the windows-rs compatible pointer to the contained `EVENT_TRACE_LOGFILEA`
     ///
     /// # Safety
     ///
-    /// This pointer is valid as long as [`Self`] is alive (and not modified elsewhere)
-    pub unsafe fn as_mut_ptr(&mut self) -> *mut Etw::EVENT_TRACE_LOGFILEA {
-        &mut self.native as *mut Etw::EVENT_TRACE_LOGFILEA
+    /// This pointer is valid as long as [`Self`] is alive (and not modified elsewhere)<br/>
+    /// Note that `OpenTraceW` **will** modify its content on output, and thus you should make sure to be the only user of this instance.
+    pub(crate) unsafe fn as_mut_ptr(&mut self) -> *mut Etw::EVENT_TRACE_LOGFILEW {
+        &mut self.native as *mut Etw::EVENT_TRACE_LOGFILEW
     }
-}
 
-impl<'tracedata> Default for EventTraceLogfile<'tracedata> {
-    fn default() -> Self {
-        unsafe { std::mem::zeroed::<EventTraceLogfile>() }
+    /// The current Context pointer.
+    pub fn context_ptr(&self) -> *const std::ffi::c_void {
+        self.native.Context
     }
 }
 

src/native/etw_types/event_record.rs

@@ -33,7 +33,7 @@ impl EventRecord {
 
     /// The `UserContext` field from the wrapped `EVENT_RECORD`
     ///
-    /// In this crate, it is always populated to point to a valid [`TraceData`](crate::trace::TraceData)
+    /// In this crate, it is always populated to point to a valid [`CallbackData`](crate::trace::CallbackData)
     pub fn user_context(&self) -> *const std::ffi::c_void {
         self.0.UserContext as *const _
     }