Remove Process-Wide Panic Hook in Headless SVG Export
Greptile flagged that
[`run_headless_svg_export`](file:///home/dietrich/Projekte/Source/turtlers/turtle-lib/src/export.rs#L80)
was setting and taking the global, process-wide panic hook without
thread coordination. In concurrent environments (such as multi-threaded
test runners or multi-threaded host applications), this caused race
conditions that could clobber custom hooks, restore stale hooks, or
capture another thread's panic hook.
[export.rs](file:///home/dietrich/Projekte/Source/turtlers/turtle-lib/src/export.rs)
1. **Removed Global Panic Hook Manipulation**:
- Removed `PanicHookFn` and `PanicHookGuard`.
- Eliminated calls to `std::panic::take_hook()` and
`std::panic::set_hook()`.
- Removed `AtomicBool` and `Arc` overhead.
2. **Inspect Panic Payload Directly in
[`run_headless_svg_export`](file:///home/dietrich/Projekte/Source/turtlers/turtle-lib/src/export.rs#L80)**:
- Evaluates user command closures inside `std::panic::catch_unwind`.
- Downcasts caught panic payload to `&str` or `String` and checks for
Macroquad assertion signatures (`THREAD_ID.is_some()` or
`macroquad`).
- Packages diagnostic guidance directly into
`ExportError::Execution(...)` without emitting unsolicited messages
to `eprintln!`.
3. **Banner in CLI Entrypoint
[`handle_svg_export`](file:///home/dietrich/Projekte/Source/turtlers/turtle-lib/src/export.rs#L137)**:
- When executed via `--export-svg`, if `run_headless_svg_export`
fails due to Macroquad window/GUI functions, formats and displays
the friendly "Headless SVG Export Note" banner to `eprintln!`.
4. **Unit Tests**:
- Added `test_headless_svg_export_success`: checks complete headless
SVG export output.
- Added `test_headless_svg_export_macroquad_panic`: verifies proper
detection and error message for simulated Macroquad panics.
- Added `test_headless_svg_export_custom_panic`: verifies that
non-Macroquad panics retain their original error message.
---
- **Default tests**:
```bash
cargo test --package turtle-lib
```
Result: 26 unit tests passed, 34 doc-tests passed (0 failures).
- **SVG feature tests**:
```bash
cargo test --package turtle-lib --features svg
```
Result: 32 unit tests passed, 34 doc-tests passed (0 failures).
- **Clippy**:
```bash
cargo clippy --package turtle-lib --features svg -- -Wclippy::pedantic
\
-Aclippy::cast_precision_loss -Aclippy::cast_sign_loss
-Aclippy::cast_possible_truncation
```
Result: 0 warnings, clean.
- **SVG export example**:
```bash
cargo run --package turtle-lib --example test_svg_export --features
svg -- --export-svg /tmp/test_output.svg
```
Result: Successfully generated valid SVG file `/tmp/test_output.svg`.
This commit is contained in:
+74
-62
@@ -72,23 +72,6 @@ pub fn parse_svg_export_arg() -> Option<String> {
|
|||||||
parse_svg_export_from_args(&mut args).ok().flatten()
|
parse_svg_export_from_args(&mut args).ok().flatten()
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(feature = "svg")]
|
|
||||||
type PanicHookFn = Box<dyn Fn(&std::panic::PanicHookInfo<'_>) + Sync + Send + 'static>;
|
|
||||||
|
|
||||||
#[cfg(feature = "svg")]
|
|
||||||
struct PanicHookGuard {
|
|
||||||
prev_hook: Option<std::sync::Arc<PanicHookFn>>,
|
|
||||||
}
|
|
||||||
|
|
||||||
#[cfg(feature = "svg")]
|
|
||||||
impl Drop for PanicHookGuard {
|
|
||||||
fn drop(&mut self) {
|
|
||||||
if let Some(prev) = self.prev_hook.take() {
|
|
||||||
std::panic::set_hook(Box::new(move |info| prev(info)));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Headless SVG export that executes drawing commands and writes an SVG file
|
/// Headless SVG export that executes drawing commands and writes an SVG file
|
||||||
/// without opening a graphics window and without calling `std::process::exit`.
|
/// without opening a graphics window and without calling `std::process::exit`.
|
||||||
///
|
///
|
||||||
@@ -102,55 +85,13 @@ where
|
|||||||
#[cfg(feature = "svg")]
|
#[cfg(feature = "svg")]
|
||||||
{
|
{
|
||||||
use std::panic::AssertUnwindSafe;
|
use std::panic::AssertUnwindSafe;
|
||||||
use std::sync::atomic::{AtomicBool, Ordering};
|
|
||||||
use std::sync::Arc;
|
|
||||||
|
|
||||||
let mut turtle = crate::create_turtle_plan();
|
let mut turtle = crate::create_turtle_plan();
|
||||||
|
|
||||||
let is_mq_panic = Arc::new(AtomicBool::new(false));
|
|
||||||
let is_mq_clone = Arc::clone(&is_mq_panic);
|
|
||||||
|
|
||||||
let prev_hook = Arc::new(std::panic::take_hook());
|
|
||||||
let prev_hook_for_closure = Arc::clone(&prev_hook);
|
|
||||||
|
|
||||||
let guard = PanicHookGuard {
|
|
||||||
prev_hook: Some(prev_hook),
|
|
||||||
};
|
|
||||||
|
|
||||||
std::panic::set_hook(Box::new(move |info| {
|
|
||||||
prev_hook_for_closure(info);
|
|
||||||
|
|
||||||
let msg = if let Some(s) = info.payload().downcast_ref::<&str>() {
|
|
||||||
*s
|
|
||||||
} else if let Some(s) = info.payload().downcast_ref::<String>() {
|
|
||||||
s.as_str()
|
|
||||||
} else {
|
|
||||||
""
|
|
||||||
};
|
|
||||||
|
|
||||||
let loc_file = info.location().map_or("", std::panic::Location::file);
|
|
||||||
let is_context_panic = msg.contains("THREAD_ID.is_some()")
|
|
||||||
|| (loc_file.contains("macroquad") && msg.contains("assertion failed"));
|
|
||||||
|
|
||||||
if is_context_panic {
|
|
||||||
is_mq_clone.store(true, Ordering::SeqCst);
|
|
||||||
eprintln!("\n================================================================================");
|
|
||||||
eprintln!("Headless SVG Export Note:");
|
|
||||||
eprintln!("A Macroquad window/rendering function (e.g. `screen_width()`, `screen_height()`,");
|
|
||||||
eprintln!("or input check) was called while running in headless export mode.");
|
|
||||||
eprintln!("Headless export does not initialize a graphics window. To resolve this:");
|
|
||||||
eprintln!(" - Use relative turtle commands or fixed coordinates instead of window queries, or");
|
|
||||||
eprintln!(" - Run the program in windowed GUI mode without the `--export-svg` flag.");
|
|
||||||
eprintln!("================================================================================\n");
|
|
||||||
}
|
|
||||||
}));
|
|
||||||
|
|
||||||
let result = std::panic::catch_unwind(AssertUnwindSafe(|| {
|
let result = std::panic::catch_unwind(AssertUnwindSafe(|| {
|
||||||
build_commands(&mut turtle);
|
build_commands(&mut turtle);
|
||||||
}));
|
}));
|
||||||
|
|
||||||
drop(guard);
|
|
||||||
|
|
||||||
match result {
|
match result {
|
||||||
Ok(()) => {
|
Ok(()) => {
|
||||||
let mut app = crate::TurtleApp::new();
|
let mut app = crate::TurtleApp::new();
|
||||||
@@ -159,12 +100,21 @@ where
|
|||||||
app.export_drawing(filename, crate::export::DrawingFormat::Svg)
|
app.export_drawing(filename, crate::export::DrawingFormat::Svg)
|
||||||
}
|
}
|
||||||
Err(payload) => {
|
Err(payload) => {
|
||||||
let err_msg = if is_mq_panic.load(Ordering::SeqCst) {
|
let raw_msg = if let Some(s) = payload.downcast_ref::<&str>() {
|
||||||
"Drawing function called Macroquad window/GUI functions (e.g. `screen_width()`, `screen_height()`) which are unavailable in headless SVG export mode.".to_string()
|
|
||||||
} else if let Some(s) = payload.downcast_ref::<&str>() {
|
|
||||||
(*s).to_string()
|
(*s).to_string()
|
||||||
} else if let Some(s) = payload.downcast_ref::<String>() {
|
} else if let Some(s) = payload.downcast_ref::<String>() {
|
||||||
s.clone()
|
s.clone()
|
||||||
|
} else {
|
||||||
|
String::new()
|
||||||
|
};
|
||||||
|
|
||||||
|
let is_context_panic = raw_msg.contains("THREAD_ID.is_some()")
|
||||||
|
|| raw_msg.contains("macroquad");
|
||||||
|
|
||||||
|
let err_msg = if is_context_panic {
|
||||||
|
"Drawing function called Macroquad window/GUI functions (e.g. `screen_width()`, `screen_height()`) which are unavailable in headless SVG export mode.".to_string()
|
||||||
|
} else if !raw_msg.is_empty() {
|
||||||
|
raw_msg
|
||||||
} else {
|
} else {
|
||||||
"Drawing function panicked during execution".to_string()
|
"Drawing function panicked during execution".to_string()
|
||||||
};
|
};
|
||||||
@@ -217,6 +167,18 @@ where
|
|||||||
std::process::exit(0);
|
std::process::exit(0);
|
||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
|
if let ExportError::Execution(ref msg) = e {
|
||||||
|
if msg.contains("unavailable in headless SVG export mode") {
|
||||||
|
eprintln!("\n================================================================================");
|
||||||
|
eprintln!("Headless SVG Export Note:");
|
||||||
|
eprintln!("A Macroquad window/rendering function (e.g. `screen_width()`, `screen_height()`,");
|
||||||
|
eprintln!("or input check) was called while running in headless export mode.");
|
||||||
|
eprintln!("Headless export does not initialize a graphics window. To resolve this:");
|
||||||
|
eprintln!(" - Use relative turtle commands or fixed coordinates instead of window queries, or");
|
||||||
|
eprintln!(" - Run the program in windowed GUI mode without the `--export-svg` flag.");
|
||||||
|
eprintln!("================================================================================\n");
|
||||||
|
}
|
||||||
|
}
|
||||||
eprintln!("Error: {e}");
|
eprintln!("Error: {e}");
|
||||||
std::process::exit(1);
|
std::process::exit(1);
|
||||||
}
|
}
|
||||||
@@ -303,4 +265,54 @@ mod tests {
|
|||||||
"SVG export feature is not enabled. Please rebuild with --features svg"
|
"SVG export feature is not enabled. Please rebuild with --features svg"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(feature = "svg")]
|
||||||
|
#[test]
|
||||||
|
fn test_headless_svg_export_success() {
|
||||||
|
use crate::Movement;
|
||||||
|
|
||||||
|
let temp_dir = std::env::temp_dir();
|
||||||
|
let path = temp_dir.join(format!("turtle_test_export_{}.svg", std::process::id()));
|
||||||
|
let path_str = path.to_str().expect("valid utf-8 path");
|
||||||
|
|
||||||
|
let res = run_headless_svg_export(|turtle| {
|
||||||
|
turtle.forward(10.0);
|
||||||
|
}, path_str);
|
||||||
|
|
||||||
|
assert!(res.is_ok());
|
||||||
|
assert!(path.exists());
|
||||||
|
let content = std::fs::read_to_string(&path).expect("read temp svg");
|
||||||
|
assert!(content.contains("<svg"));
|
||||||
|
let _ = std::fs::remove_file(&path);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(feature = "svg")]
|
||||||
|
#[test]
|
||||||
|
fn test_headless_svg_export_macroquad_panic() {
|
||||||
|
let res = run_headless_svg_export(|_turtle| {
|
||||||
|
panic!("assertion failed: THREAD_ID.is_some()");
|
||||||
|
}, "unused.svg");
|
||||||
|
|
||||||
|
match res {
|
||||||
|
Err(ExportError::Execution(msg)) => {
|
||||||
|
assert!(msg.contains("unavailable in headless SVG export mode"));
|
||||||
|
}
|
||||||
|
other => panic!("expected ExportError::Execution, got {other:?}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(feature = "svg")]
|
||||||
|
#[test]
|
||||||
|
fn test_headless_svg_export_custom_panic() {
|
||||||
|
let res = run_headless_svg_export(|_turtle| {
|
||||||
|
panic!("custom user panic occurred");
|
||||||
|
}, "unused.svg");
|
||||||
|
|
||||||
|
match res {
|
||||||
|
Err(ExportError::Execution(msg)) => {
|
||||||
|
assert_eq!(msg, "custom user panic occurred");
|
||||||
|
}
|
||||||
|
other => panic!("expected ExportError::Execution, got {other:?}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user