Backport Validation Error Related Problems

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Change Log
 
+## wgpu-hal 0.12.5 (2022-04-19)
+  - fix crashes when logging in debug message callbacks
+  - fix program termination when dx12 or gles error messages happen.
+  - implement validation canary
+  - DX12:
+    - Ignore erroneous validation error from DXGI debug layer. 
+
 ## wgpu-hal-0.12.4 (2022-01-24)
   - Metal:
     - check for MSL-2.3

wgpu-hal/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "wgpu-hal"
-version = "0.12.4"
+version = "0.12.5"
 authors = ["wgpu developers"]
 edition = "2018"
 description = "WebGPU hardware abstraction layer"

wgpu-hal/src/dx12/instance.rs

@@ -58,11 +58,26 @@ unsafe extern "system" fn output_debug_string_handler(
         return excpt::EXCEPTION_CONTINUE_SEARCH;
     }
 
-    log::log!(level, "{}", message);
+    let dxgi_debug_layer_bug_text1 = "ID3D12CommandQueue::Present";
+    // We currently dont cross command list types, so this text has the
+    // highest chance of being a true positive for this issue.
+    let dxgi_debug_layer_bug_text2 = "resource state on previous Command List type";
+    if level == log::Level::Error
+        && message.contains(dxgi_debug_layer_bug_text1)
+        && message.contains(dxgi_debug_layer_bug_text2)
+    {
+        // This is a bug in the DXGI and D3D12 validation layer when on windows 11.
+        // See https://stackoverflow.com/q/69805245 for more.
+        return excpt::EXCEPTION_CONTINUE_SEARCH;
+    }
+
+    let _ = std::panic::catch_unwind(|| {
+        log::log!(level, "{}", message);
+    });
 
     if cfg!(debug_assertions) && level == log::Level::Error {
-        // Panicking behind FFI is UB, so we just exit.
-        std::process::exit(1);
+        // Set canary and continue
+        crate::VALIDATION_CANARY.set();
     }
 
     excpt::EXCEPTION_CONTINUE_EXECUTION

wgpu-hal/src/gles/egl.rs

@@ -247,17 +247,20 @@ fn gl_debug_message_callback(source: u32, gltype: u32, id: u32, severity: u32, m
         _ => unreachable!(),
     };
 
-    log::log!(
-        log_severity,
-        "GLES: [{}/{}] ID {} : {}",
-        source_str,
-        type_str,
-        id,
-        message
-    );
+    let _ = std::panic::catch_unwind(|| {
+        log::log!(
+            log_severity,
+            "GLES: [{}/{}] ID {} : {}",
+            source_str,
+            type_str,
+            id,
+            message
+        );
+    });
 
     if cfg!(debug_assertions) && log_severity == log::Level::Error {
-        std::process::exit(1);
+        // Set canary and continue
+        crate::VALIDATION_CANARY.set();
     }
 }
 

wgpu-hal/src/lib.rs

@@ -84,6 +84,7 @@ use std::{
     num::{NonZeroU32, NonZeroU8},
     ops::{Range, RangeInclusive},
     ptr::NonNull,
+    sync::atomic::AtomicBool,
 };
 
 use bitflags::bitflags;
@@ -1143,6 +1144,39 @@ pub struct ComputePassDescriptor<'a> {
     pub label: Label<'a>,
 }
 
+/// Stores if any API validation error has occurred in this process
+/// since it was last reset.
+///
+/// This is used for internal wgpu testing only and _must not_ be used
+/// as a way to check for errors.
+///
+/// This works as a static because `cargo nextest` runs all of our
+/// tests in separate processes, so each test gets its own canary.
+///
+/// This prevents the issue of one validation error terminating the
+/// entire process.
+pub static VALIDATION_CANARY: ValidationCanary = ValidationCanary {
+    inner: AtomicBool::new(false),
+};
+
+/// Flag for internal testing.
+pub struct ValidationCanary {
+    inner: AtomicBool,
+}
+
+impl ValidationCanary {
+    #[allow(dead_code)] // in some configurations this function is dead
+    fn set(&self) {
+        self.inner.store(true, std::sync::atomic::Ordering::SeqCst);
+    }
+
+    /// Returns true if any API validation error has occurred in this process
+    /// since the last call to this function.
+    pub fn get_and_reset(&self) -> bool {
+        self.inner.swap(false, std::sync::atomic::Ordering::SeqCst)
+    }
+}
+
 #[test]
 fn test_default_limits() {
     let limits = wgt::Limits::default();

wgpu-hal/src/vulkan/instance.rs

@@ -44,14 +44,16 @@ unsafe extern "system" fn debug_utils_messenger_callback(
         CStr::from_ptr(cd.p_message).to_string_lossy()
     };
 
-    log::log!(
-        level,
-        "{:?} [{} (0x{:x})]\n\t{}",
-        message_type,
-        message_id_name,
-        cd.message_id_number,
-        message,
-    );
+    let _ = std::panic::catch_unwind(|| {
+        log::log!(
+            level,
+            "{:?} [{} (0x{:x})]\n\t{}",
+            message_type,
+            message_id_name,
+            cd.message_id_number,
+            message,
+        );
+    });
 
     if cd.queue_label_count != 0 {
         let labels = slice::from_raw_parts(cd.p_queue_labels, cd.queue_label_count as usize);
@@ -64,7 +66,10 @@ unsafe extern "system" fn debug_utils_messenger_callback(
                     .map(|lbl| CStr::from_ptr(lbl).to_string_lossy())
             })
             .collect::<Vec<_>>();
-        log::log!(level, "\tqueues: {}", names.join(", "));
+
+        let _ = std::panic::catch_unwind(|| {
+            log::log!(level, "\tqueues: {}", names.join(", "));
+        });
     }
 
     if cd.cmd_buf_label_count != 0 {
@@ -78,7 +83,10 @@ unsafe extern "system" fn debug_utils_messenger_callback(
                     .map(|lbl| CStr::from_ptr(lbl).to_string_lossy())
             })
             .collect::<Vec<_>>();
-        log::log!(level, "\tcommand buffers: {}", names.join(", "));
+
+        let _ = std::panic::catch_unwind(|| {
+            log::log!(level, "\tcommand buffers: {}", names.join(", "));
+        });
     }
 
     if cd.object_count != 0 {
@@ -99,7 +107,14 @@ unsafe extern "system" fn debug_utils_messenger_callback(
                 )
             })
             .collect::<Vec<_>>();
-        log::log!(level, "\tobjects: {}", names.join(", "));
+        let _ = std::panic::catch_unwind(|| {
+            log::log!(level, "\tobjects: {}", names.join(", "));
+        });
+    }
+
+    if cfg!(debug_assertions) && level == log::Level::Error {
+        // Set canary and continue
+        crate::VALIDATION_CANARY.set();
     }
 
     vk::FALSE