Skip to content

Commit 1b55652

Browse files
authored
feat(embed): correctly handle panic inside embed (#272)
1 parent 1a8211c commit 1b55652

File tree

4 files changed

+41
-11
lines changed

4 files changed

+41
-11
lines changed

src/embed/embed.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,14 @@
22

33
// We actually use the PHP embed API to run PHP code in test
44
// At some point we might want to use our own SAPI to do that
5-
void ext_php_rs_embed_callback(int argc, char** argv, void (*callback)(void *), void *ctx) {
5+
void* ext_php_rs_embed_callback(int argc, char** argv, void* (*callback)(void *), void *ctx) {
6+
void *result;
7+
68
PHP_EMBED_START_BLOCK(argc, argv)
79

8-
callback(ctx);
10+
result = callback(ctx);
911

1012
PHP_EMBED_END_BLOCK()
11-
}
13+
14+
return result;
15+
}

src/embed/embed.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
#include "zend.h"
22
#include "sapi/embed/php_embed.h"
33

4-
void ext_php_rs_embed_callback(int argc, char** argv, void (*callback)(void *), void *ctx);
4+
void* ext_php_rs_embed_callback(int argc, char** argv, void* (*callback)(void *), void *ctx);

src/embed/ffi.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ extern "C" {
1010
pub fn ext_php_rs_embed_callback(
1111
argc: c_int,
1212
argv: *mut *mut c_char,
13-
func: unsafe extern "C" fn(*const c_void),
13+
func: unsafe extern "C" fn(*const c_void) -> *mut c_void,
1414
ctx: *const c_void,
15-
);
15+
) -> *mut c_void;
1616
}

src/embed/mod.rs

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use crate::types::{ZendObject, Zval};
1616
use crate::zend::ExecutorGlobals;
1717
use parking_lot::{const_rwlock, RwLock};
1818
use std::ffi::{c_char, c_void, CString, NulError};
19+
use std::panic::{catch_unwind, resume_unwind, RefUnwindSafe};
1920
use std::path::Path;
2021
use std::ptr::null_mut;
2122

@@ -104,24 +105,41 @@ impl Embed {
104105
/// assert_eq!(foo.unwrap().string().unwrap(), "foo");
105106
/// });
106107
/// ```
107-
pub fn run<F: Fn()>(func: F) {
108+
pub fn run<F: Fn() + RefUnwindSafe>(func: F) {
108109
// @TODO handle php thread safe
109110
//
110111
// This is to prevent multiple threads from running php at the same time
111112
// At some point we should detect if php is compiled with thread safety and avoid doing that in this case
112113
let _guard = RUN_FN_LOCK.write();
113114

114-
unsafe extern "C" fn wrapper<F: Fn()>(ctx: *const c_void) {
115-
(*(ctx as *const F))();
115+
unsafe extern "C" fn wrapper<F: Fn() + RefUnwindSafe>(ctx: *const c_void) -> *mut c_void {
116+
// we try to catch panic here so we correctly shutdown php if it happens
117+
// mandatory when we do assert on test as other test would not run correctly
118+
let panic = catch_unwind(|| {
119+
(*(ctx as *const F))();
120+
});
121+
122+
let panic_ptr = Box::into_raw(Box::new(panic));
123+
124+
panic_ptr as *mut c_void
116125
}
117126

118-
unsafe {
127+
let panic = unsafe {
119128
ext_php_rs_embed_callback(
120129
0,
121130
null_mut(),
122131
wrapper::<F>,
123132
&func as *const F as *const c_void,
124-
);
133+
)
134+
};
135+
136+
if panic.is_null() {
137+
return;
138+
}
139+
140+
if let Err(err) = unsafe { *Box::from_raw(panic as *mut std::thread::Result<()>) } {
141+
// we resume the panic here so it can be catched correctly by the test framework
142+
resume_unwind(err);
125143
}
126144
}
127145

@@ -218,4 +236,12 @@ mod tests {
218236
assert!(!result.is_ok());
219237
});
220238
}
239+
240+
#[test]
241+
#[should_panic]
242+
fn test_panic() {
243+
Embed::run(|| {
244+
panic!("test panic");
245+
});
246+
}
221247
}

0 commit comments

Comments
 (0)