Skip to content

Commit 530e66e

Browse files
kakserpomkakserpom
committed
fix(zend_bailout): Fix zend_bailout handling #537 
1 parent 36a1811 commit 530e66e

File tree

15 files changed

+669
-52
lines changed

15 files changed

+669
-52
lines changed

‎crates/macros/src/function.rs‎

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,20 @@ impl<'a> Function<'a> {
291291
ex: &mut ::ext_php_rs::zend::ExecuteData,
292292
retval: &mut ::ext_php_rs::types::Zval,
293293
) {
294-
#handler_body
294+
use ::ext_php_rs::zend::try_catch;
295+
use ::std::panic::AssertUnwindSafe;
296+
297+
// Wrap the handler body with try_catch to ensure Rust destructors
298+
// are called if a bailout occurs (issue #537)
299+
let catch_result = try_catch(AssertUnwindSafe(|| {
300+
#handler_body
301+
}));
302+
303+
// If there was a bailout, run BailoutGuard cleanups and re-trigger
304+
if catch_result.is_err() {
305+
::ext_php_rs::zend::run_bailout_cleanups();
306+
unsafe { ::ext_php_rs::zend::bailout(); }
307+
}
295308
}
296309
}
297310
handler
@@ -535,18 +548,34 @@ impl<'a> Function<'a> {
535548
::ext_php_rs::class::ConstructorMeta {
536549
constructor: {
537550
fn inner(ex: &mut ::ext_php_rs::zend::ExecuteData) -> ::ext_php_rs::class::ConstructorResult<#class> {
538-
#(#arg_declarations)*
539-
let parse = ex.parser()
540-
#(.arg(&mut #required_arg_names))*
541-
.not_required()
542-
#(.arg(&mut #not_required_arg_names))*
543-
#variadic
544-
.parse();
545-
if parse.is_err() {
546-
return ::ext_php_rs::class::ConstructorResult::ArgError;
551+
use ::ext_php_rs::zend::try_catch;
552+
use ::std::panic::AssertUnwindSafe;
553+
554+
// Wrap the constructor body with try_catch to ensure Rust destructors
555+
// are called if a bailout occurs (issue #537)
556+
let catch_result = try_catch(AssertUnwindSafe(|| {
557+
#(#arg_declarations)*
558+
let parse = ex.parser()
559+
#(.arg(&mut #required_arg_names))*
560+
.not_required()
561+
#(.arg(&mut #not_required_arg_names))*
562+
#variadic
563+
.parse();
564+
if parse.is_err() {
565+
return ::ext_php_rs::class::ConstructorResult::ArgError;
566+
}
567+
#(#variadic_bindings)*
568+
#class::#ident(#({#arg_accessors}),*).into()
569+
}));
570+
571+
// If there was a bailout, run BailoutGuard cleanups and re-trigger
572+
match catch_result {
573+
Ok(result) => result,
574+
Err(_) => {
575+
::ext_php_rs::zend::run_bailout_cleanups();
576+
unsafe { ::ext_php_rs::zend::bailout() }
577+
}
547578
}
548-
#(#variadic_bindings)*
549-
#class::#ident(#({#arg_accessors}),*).into()
550579
}
551580
inner
552581
},

‎src/builders/class.rs‎

Lines changed: 35 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -219,31 +219,43 @@ impl ClassBuilder {
219219

220220
zend_fastcall! {
221221
extern fn constructor<T: RegisteredClass>(ex: &mut ExecuteData, _: &mut Zval) {
222-
let Some(ConstructorMeta { constructor, .. }) = T::constructor() else {
223-
PhpException::default("You cannot instantiate this class from PHP.".into())
224-
.throw()
225-
.expect("Failed to throw exception when constructing class");
226-
return;
227-
};
228-
229-
let this = match constructor(ex) {
230-
ConstructorResult::Ok(this) => this,
231-
ConstructorResult::Exception(e) => {
232-
e.throw()
222+
use crate::zend::try_catch;
223+
use std::panic::AssertUnwindSafe;
224+
225+
// Wrap the constructor body with try_catch to ensure Rust destructors
226+
// are called if a bailout occurs (issue #537)
227+
let catch_result = try_catch(AssertUnwindSafe(|| {
228+
let Some(ConstructorMeta { constructor, .. }) = T::constructor() else {
229+
PhpException::default("You cannot instantiate this class from PHP.".into())
230+
.throw()
231+
.expect("Failed to throw exception when constructing class");
232+
return;
233+
};
234+
235+
let this = match constructor(ex) {
236+
ConstructorResult::Ok(this) => this,
237+
ConstructorResult::Exception(e) => {
238+
e.throw()
239+
.expect("Failed to throw exception while constructing class");
240+
return;
241+
}
242+
ConstructorResult::ArgError => return,
243+
};
244+
245+
let Some(this_obj) = ex.get_object::<T>() else {
246+
PhpException::default("Failed to retrieve reference to `this` object.".into())
247+
.throw()
233248
.expect("Failed to throw exception while constructing class");
234249
return;
235-
}
236-
ConstructorResult::ArgError => return,
237-
};
238-
239-
let Some(this_obj) = ex.get_object::<T>() else {
240-
PhpException::default("Failed to retrieve reference to `this` object.".into())
241-
.throw()
242-
.expect("Failed to throw exception while constructing class");
243-
return;
244-
};
245-
246-
this_obj.initialize(this);
250+
};
251+
252+
this_obj.initialize(this);
253+
}));
254+
255+
// If there was a bailout, re-trigger it after Rust cleanup
256+
if catch_result.is_err() {
257+
unsafe { crate::zend::bailout(); }
258+
}
247259
}
248260
}
249261

‎src/embed/mod.rs‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use crate::types::{ZendObject, Zval};
1717
use crate::zend::{ExecutorGlobals, panic_wrapper, try_catch};
1818
use parking_lot::{RwLock, const_rwlock};
1919
use std::ffi::{CString, NulError, c_char, c_void};
20-
use std::panic::{RefUnwindSafe, resume_unwind};
20+
use std::panic::{AssertUnwindSafe, UnwindSafe, resume_unwind};
2121
use std::path::Path;
2222
use std::ptr::null_mut;
2323

@@ -105,7 +105,9 @@ impl Embed {
105105
zend_stream_init_filename(&raw mut file_handle, path.as_ptr());
106106
}
107107

108-
let exec_result = try_catch(|| unsafe { php_execute_script(&raw mut file_handle) });
108+
let exec_result = try_catch(AssertUnwindSafe(|| unsafe {
109+
php_execute_script(&raw mut file_handle)
110+
}));
109111

110112
unsafe { zend_destroy_file_handle(&raw mut file_handle) }
111113

@@ -141,7 +143,7 @@ impl Embed {
141143
/// assert_eq!(foo.unwrap().string().unwrap(), "foo");
142144
/// });
143145
/// ```
144-
pub fn run<R, F: FnMut() -> R + RefUnwindSafe>(func: F) -> R
146+
pub fn run<R, F: FnOnce() -> R + UnwindSafe>(func: F) -> R
145147
where
146148
R: Default,
147149
{
@@ -161,6 +163,9 @@ impl Embed {
161163
)
162164
};
163165

166+
// Prevent the closure from being dropped here since it was consumed in panic_wrapper
167+
std::mem::forget(func);
168+
164169
// This can happen if there is a bailout
165170
if panic.is_null() {
166171
return R::default();
@@ -206,13 +211,13 @@ impl Embed {
206211

207212
let mut result = Zval::new();
208213

209-
let exec_result = try_catch(|| unsafe {
214+
let exec_result = try_catch(AssertUnwindSafe(|| unsafe {
210215
zend_eval_string(
211216
cstr.as_ptr().cast::<c_char>(),
212217
&raw mut result,
213218
c"run".as_ptr().cast(),
214219
)
215-
});
220+
}));
216221

217222
match exec_result {
218223
Err(_) => Err(EmbedError::CatchError),

‎src/lib.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ pub mod prelude {
5656
pub use crate::php_println;
5757
pub use crate::php_write;
5858
pub use crate::types::ZendCallable;
59+
pub use crate::zend::BailoutGuard;
5960
pub use crate::{
6061
ZvalConvert, php_class, php_const, php_extern, php_function, php_impl, php_interface,
6162
php_module, wrap_constant, wrap_function, zend_fastcall,

0 commit comments

Comments
 (0)