From 1b55652b7ab51f37168970f68832412550ebea30 Mon Sep 17 00:00:00 2001 From: Joel Wurtz Date: Fri, 20 Oct 2023 21:52:52 +0200 Subject: [PATCH] feat(embed): correctly handle panic inside embed (#272) --- src/embed/embed.c | 10 +++++++--- src/embed/embed.h | 2 +- src/embed/ffi.rs | 4 ++-- src/embed/mod.rs | 36 +++++++++++++++++++++++++++++++----- 4 files changed, 41 insertions(+), 11 deletions(-) diff --git a/src/embed/embed.c b/src/embed/embed.c index 6aaa30c..d8b3a78 100644 --- a/src/embed/embed.c +++ b/src/embed/embed.c @@ -2,10 +2,14 @@ // We actually use the PHP embed API to run PHP code in test // At some point we might want to use our own SAPI to do that -void ext_php_rs_embed_callback(int argc, char** argv, void (*callback)(void *), void *ctx) { +void* ext_php_rs_embed_callback(int argc, char** argv, void* (*callback)(void *), void *ctx) { + void *result; + PHP_EMBED_START_BLOCK(argc, argv) - callback(ctx); + result = callback(ctx); PHP_EMBED_END_BLOCK() -} \ No newline at end of file + + return result; +} diff --git a/src/embed/embed.h b/src/embed/embed.h index 910dcd9..beabffb 100644 --- a/src/embed/embed.h +++ b/src/embed/embed.h @@ -1,4 +1,4 @@ #include "zend.h" #include "sapi/embed/php_embed.h" -void ext_php_rs_embed_callback(int argc, char** argv, void (*callback)(void *), void *ctx); +void* ext_php_rs_embed_callback(int argc, char** argv, void* (*callback)(void *), void *ctx); diff --git a/src/embed/ffi.rs b/src/embed/ffi.rs index 74124a9..3a1f6d7 100644 --- a/src/embed/ffi.rs +++ b/src/embed/ffi.rs @@ -10,7 +10,7 @@ extern "C" { pub fn ext_php_rs_embed_callback( argc: c_int, argv: *mut *mut c_char, - func: unsafe extern "C" fn(*const c_void), + func: unsafe extern "C" fn(*const c_void) -> *mut c_void, ctx: *const c_void, - ); + ) -> *mut c_void; } diff --git a/src/embed/mod.rs b/src/embed/mod.rs index 93e973b..581e385 100644 --- a/src/embed/mod.rs +++ b/src/embed/mod.rs @@ -16,6 +16,7 @@ use crate::types::{ZendObject, Zval}; use crate::zend::ExecutorGlobals; use parking_lot::{const_rwlock, RwLock}; use std::ffi::{c_char, c_void, CString, NulError}; +use std::panic::{catch_unwind, resume_unwind, RefUnwindSafe}; use std::path::Path; use std::ptr::null_mut; @@ -104,24 +105,41 @@ impl Embed { /// assert_eq!(foo.unwrap().string().unwrap(), "foo"); /// }); /// ``` - pub fn run(func: F) { + pub fn run(func: F) { // @TODO handle php thread safe // // This is to prevent multiple threads from running php at the same time // At some point we should detect if php is compiled with thread safety and avoid doing that in this case let _guard = RUN_FN_LOCK.write(); - unsafe extern "C" fn wrapper(ctx: *const c_void) { - (*(ctx as *const F))(); + unsafe extern "C" fn wrapper(ctx: *const c_void) -> *mut c_void { + // we try to catch panic here so we correctly shutdown php if it happens + // mandatory when we do assert on test as other test would not run correctly + let panic = catch_unwind(|| { + (*(ctx as *const F))(); + }); + + let panic_ptr = Box::into_raw(Box::new(panic)); + + panic_ptr as *mut c_void } - unsafe { + let panic = unsafe { ext_php_rs_embed_callback( 0, null_mut(), wrapper::, &func as *const F as *const c_void, - ); + ) + }; + + if panic.is_null() { + return; + } + + if let Err(err) = unsafe { *Box::from_raw(panic as *mut std::thread::Result<()>) } { + // we resume the panic here so it can be catched correctly by the test framework + resume_unwind(err); } } @@ -218,4 +236,12 @@ mod tests { assert!(!result.is_ok()); }); } + + #[test] + #[should_panic] + fn test_panic() { + Embed::run(|| { + panic!("test panic"); + }); + } }