Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(embed): correctly handle panic inside embed #272

Merged
merged 1 commit into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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");
});
}
}