Skip to content

Commit

Permalink
feat(embed): correctly handle panic inside embed (#272)
Browse files Browse the repository at this point in the history
  • Loading branch information
joelwurtz authored Oct 20, 2023
1 parent 1a8211c commit 1b55652
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 11 deletions.
10 changes: 7 additions & 3 deletions src/embed/embed.c
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

return result;
}
2 changes: 1 addition & 1 deletion src/embed/embed.h
Original file line number Diff line number Diff line change
@@ -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);
4 changes: 2 additions & 2 deletions src/embed/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
36 changes: 31 additions & 5 deletions src/embed/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -104,24 +105,41 @@ impl Embed {
/// assert_eq!(foo.unwrap().string().unwrap(), "foo");
/// });
/// ```
pub fn run<F: Fn()>(func: F) {
pub fn run<F: Fn() + RefUnwindSafe>(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<F: Fn()>(ctx: *const c_void) {
(*(ctx as *const F))();
unsafe extern "C" fn wrapper<F: Fn() + RefUnwindSafe>(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::<F>,
&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);
}
}

Expand Down Expand Up @@ -218,4 +236,12 @@ mod tests {
assert!(!result.is_ok());
});
}

#[test]
#[should_panic]
fn test_panic() {
Embed::run(|| {
panic!("test panic");
});
}
}

0 comments on commit 1b55652

Please sign in to comment.