fix(zend_bailout): Fix zend_bailout handling #537

crates/macros/src/function.rs

@@ -291,7 +291,20 @@ impl<'a> Function<'a> {
                         ex: &mut ::ext_php_rs::zend::ExecuteData,
                         retval: &mut ::ext_php_rs::types::Zval,
                     ) {
-                        #handler_body
+                        use ::ext_php_rs::zend::try_catch;
+                        use ::std::panic::AssertUnwindSafe;
+
+                        // Wrap the handler body with try_catch to ensure Rust destructors
+                        // are called if a bailout occurs (issue #537)
+                        let catch_result = try_catch(AssertUnwindSafe(|| {
+                            #handler_body
+                        }));
+
+                        // If there was a bailout, run BailoutGuard cleanups and re-trigger
+                        if catch_result.is_err() {
+                            ::ext_php_rs::zend::run_bailout_cleanups();
+                            unsafe { ::ext_php_rs::zend::bailout(); }
+                        }
                     }
                 }
                 handler
@@ -535,18 +548,34 @@ impl<'a> Function<'a> {
             ::ext_php_rs::class::ConstructorMeta {
                 constructor: {
                     fn inner(ex: &mut ::ext_php_rs::zend::ExecuteData) -> ::ext_php_rs::class::ConstructorResult<#class> {
-                        #(#arg_declarations)*
-                        let parse = ex.parser()
-                            #(.arg(&mut #required_arg_names))*
-                            .not_required()
-                            #(.arg(&mut #not_required_arg_names))*
-                            #variadic
-                            .parse();
-                        if parse.is_err() {
-                            return ::ext_php_rs::class::ConstructorResult::ArgError;
+                        use ::ext_php_rs::zend::try_catch;
+                        use ::std::panic::AssertUnwindSafe;
+
+                        // Wrap the constructor body with try_catch to ensure Rust destructors
+                        // are called if a bailout occurs (issue #537)
+                        let catch_result = try_catch(AssertUnwindSafe(|| {
+                            #(#arg_declarations)*
+                            let parse = ex.parser()
+                                #(.arg(&mut #required_arg_names))*
+                                .not_required()
+                                #(.arg(&mut #not_required_arg_names))*
+                                #variadic
+                                .parse();
+                            if parse.is_err() {
+                                return ::ext_php_rs::class::ConstructorResult::ArgError;
+                            }
+                            #(#variadic_bindings)*
+                            #class::#ident(#({#arg_accessors}),*).into()
+                        }));
+
+                        // If there was a bailout, run BailoutGuard cleanups and re-trigger
+                        match catch_result {
+                            Ok(result) => result,
+                            Err(_) => {
+                                ::ext_php_rs::zend::run_bailout_cleanups();
+                                unsafe { ::ext_php_rs::zend::bailout() }
+                            }
                         }
-                        #(#variadic_bindings)*
-                        #class::#ident(#({#arg_accessors}),*).into()
                     }
                     inner
                 },

guide/src/SUMMARY.md

@@ -42,6 +42,7 @@
 # Advanced Topics
 
 - [Async](./advanced/async_impl.md)
+- [Bailout Guard](./advanced/bailout_guard.md)
 - [Allowed Bindings](./advanced/allowed_bindings.md)
 
 # Migration Guides

guide/src/advanced/bailout_guard.md

@@ -0,0 +1,147 @@
+# Bailout Guard
+
+When PHP triggers a "bailout" (via `exit()`, `die()`, or a fatal error), it uses
+`longjmp` to unwind the stack. This bypasses Rust's normal drop semantics,
+meaning destructors for stack-allocated values won't run. This can lead to
+resource leaks for things like file handles, network connections, or locks.
+
+## The Problem
+
+Consider this code:
+
+```rust,ignore
+#[php_function]
+pub fn process_file(callback: ZendCallable) {
+    let file = File::open("data.txt").unwrap();
+
+    // If callback calls exit(), the file handle leaks!
+    callback.try_call(vec![]);
+
+    // file.drop() never runs
+}
+```
+
+If the PHP callback triggers `exit()`, the `File` handle is never closed because
+`longjmp` skips Rust's destructor calls.
+
+## Solution 1: Using `try_call`
+
+The simplest solution is to use `try_call` for PHP callbacks. It catches bailouts
+internally and returns normally, allowing Rust destructors to run:
+
+```rust,ignore
+#[php_function]
+pub fn process_file(callback: ZendCallable) {
+    let file = File::open("data.txt").unwrap();
+
+    // try_call catches bailout, function returns, file is dropped
+    let result = callback.try_call(vec![]);
+
+    if result.is_err() {
+        // Bailout occurred, but file will still be closed
+        // when this function returns
+    }
+}
+```
+
+## Solution 2: Using `BailoutGuard`
+
+For cases where you need guaranteed cleanup even if bailout occurs directly
+(not through `try_call`), use `BailoutGuard`:
+
+```rust,ignore
+use ext_php_rs::prelude::*;
+use std::fs::File;
+
+#[php_function]
+pub fn process_file(callback: ZendCallable) {
+    // Wrap the file handle in BailoutGuard
+    let file = BailoutGuard::new(File::open("data.txt").unwrap());
+
+    // Even if bailout occurs, the file will be closed
+    callback.try_call(vec![]);
+
+    // Use the file via Deref
+    // file.read_to_string(...);
+}
+```
+
+### How `BailoutGuard` Works
+
+1. **Heap allocation**: The wrapped value is heap-allocated so it survives
+   the `longjmp` stack unwinding.
+
+2. **Cleanup registration**: A cleanup callback is registered in thread-local
+   storage when the guard is created.
+
+3. **On normal drop**: The cleanup is cancelled and the value is dropped normally.
+
+4. **On bailout**: Before re-triggering the bailout, all registered cleanup
+   callbacks are executed, dropping the guarded values.
+
+### API
+
+```rust,ignore
+// Create a guard
+let guard = BailoutGuard::new(value);
+
+// Access the value (implements Deref and DerefMut)
+guard.do_something();
+let inner: &T = &*guard;
+let inner_mut: &mut T = &mut *guard;
+
+// Explicitly get references
+let inner: &T = guard.get();
+let inner_mut: &mut T = guard.get_mut();
+
+// Extract the value, cancelling cleanup
+let value: T = guard.into_inner();
+```
+
+### Performance Note
+
+`BailoutGuard` incurs a heap allocation. Only use it for values that absolutely
+must be cleaned up, such as:
+
+- File handles
+- Network connections
+- Database connections
+- Locks and mutexes
+- Other system resources
+
+For simple values without cleanup requirements, the overhead isn't worth it.
+
+## Nested Calls
+
+`BailoutGuard` works correctly with nested function calls. Guards at all
+nesting levels are cleaned up when bailout occurs:
+
+```rust,ignore
+#[php_function]
+pub fn outer_function(callback: ZendCallable) {
+    let _outer_resource = BailoutGuard::new(Resource::new());
+
+    inner_function(&callback);
+}
+
+fn inner_function(callback: &ZendCallable) {
+    let _inner_resource = BailoutGuard::new(Resource::new());
+
+    // If bailout occurs here, both inner and outer resources are cleaned up
+    callback.try_call(vec![]);
+}
+```
+
+## Best Practices
+
+1. **Prefer `try_call`**: For most cases, using `try_call` and handling the
+   error result is simpler and doesn't require heap allocation.
+
+2. **Use `BailoutGuard` for critical resources**: Only wrap values that
+   absolutely must be cleaned up (connections, locks, etc.).
+
+3. **Don't overuse**: Not every value needs to be wrapped. Simple data
+   structures without cleanup requirements don't need `BailoutGuard`.
+
+4. **Combine approaches**: Use `try_call` where possible and `BailoutGuard`
+   for critical resources that must be cleaned up regardless.

src/builders/class.rs

@@ -219,31 +219,43 @@ impl ClassBuilder {
 
         zend_fastcall! {
             extern fn constructor<T: RegisteredClass>(ex: &mut ExecuteData, _: &mut Zval) {
-                let Some(ConstructorMeta { constructor, .. }) = T::constructor() else {
-                    PhpException::default("You cannot instantiate this class from PHP.".into())
-                        .throw()
-                        .expect("Failed to throw exception when constructing class");
-                    return;
-                };
-
-                let this = match constructor(ex) {
-                    ConstructorResult::Ok(this) => this,
-                    ConstructorResult::Exception(e) => {
-                        e.throw()
+                use crate::zend::try_catch;
+                use std::panic::AssertUnwindSafe;
+
+                // Wrap the constructor body with try_catch to ensure Rust destructors
+                // are called if a bailout occurs (issue #537)
+                let catch_result = try_catch(AssertUnwindSafe(|| {
+                    let Some(ConstructorMeta { constructor, .. }) = T::constructor() else {
+                        PhpException::default("You cannot instantiate this class from PHP.".into())
+                            .throw()
+                            .expect("Failed to throw exception when constructing class");
+                        return;
+                    };
+
+                    let this = match constructor(ex) {
+                        ConstructorResult::Ok(this) => this,
+                        ConstructorResult::Exception(e) => {
+                            e.throw()
+                                .expect("Failed to throw exception while constructing class");
+                            return;
+                        }
+                        ConstructorResult::ArgError => return,
+                    };
+
+                    let Some(this_obj) = ex.get_object::<T>() else {
+                        PhpException::default("Failed to retrieve reference to `this` object.".into())
+                            .throw()
                             .expect("Failed to throw exception while constructing class");
                         return;
-                    }
-                    ConstructorResult::ArgError => return,
-                };
-
-                let Some(this_obj) = ex.get_object::<T>() else {
-                    PhpException::default("Failed to retrieve reference to `this` object.".into())
-                        .throw()
-                        .expect("Failed to throw exception while constructing class");
-                    return;
-                };
-
-                this_obj.initialize(this);
+                    };
+
+                    this_obj.initialize(this);
+                }));
+
+                // If there was a bailout, re-trigger it after Rust cleanup
+                if catch_result.is_err() {
+                    unsafe { crate::zend::bailout(); }
+                }
             }
         }
 

src/embed/mod.rs

@@ -17,7 +17,7 @@ use crate::types::{ZendObject, Zval};
 use crate::zend::{ExecutorGlobals, panic_wrapper, try_catch};
 use parking_lot::{RwLock, const_rwlock};
 use std::ffi::{CString, NulError, c_char, c_void};
-use std::panic::{RefUnwindSafe, resume_unwind};
+use std::panic::{AssertUnwindSafe, UnwindSafe, resume_unwind};
 use std::path::Path;
 use std::ptr::null_mut;
 
@@ -105,7 +105,9 @@ impl Embed {
             zend_stream_init_filename(&raw mut file_handle, path.as_ptr());
         }
 
-        let exec_result = try_catch(|| unsafe { php_execute_script(&raw mut file_handle) });
+        let exec_result = try_catch(AssertUnwindSafe(|| unsafe {
+            php_execute_script(&raw mut file_handle)
+        }));
 
         unsafe { zend_destroy_file_handle(&raw mut file_handle) }
 
@@ -141,7 +143,7 @@ impl Embed {
     ///    assert_eq!(foo.unwrap().string().unwrap(), "foo");
     /// });
     /// ```
-    pub fn run<R, F: FnMut() -> R + RefUnwindSafe>(func: F) -> R
+    pub fn run<R, F: FnOnce() -> R + UnwindSafe>(func: F) -> R
     where
         R: Default,
     {
@@ -161,6 +163,9 @@ impl Embed {
             )
         };
 
+        // Prevent the closure from being dropped here since it was consumed in panic_wrapper
+        std::mem::forget(func);
+
         // This can happen if there is a bailout
         if panic.is_null() {
             return R::default();
@@ -206,13 +211,13 @@ impl Embed {
 
         let mut result = Zval::new();
 
-        let exec_result = try_catch(|| unsafe {
+        let exec_result = try_catch(AssertUnwindSafe(|| unsafe {
             zend_eval_string(
                 cstr.as_ptr().cast::<c_char>(),
                 &raw mut result,
                 c"run".as_ptr().cast(),
             )
-        });
+        }));
 
         match exec_result {
             Err(_) => Err(EmbedError::CatchError),

src/lib.rs

@@ -56,6 +56,7 @@ pub mod prelude {
     pub use crate::php_println;
     pub use crate::php_write;
     pub use crate::types::ZendCallable;
+    pub use crate::zend::BailoutGuard;
     pub use crate::{
         ZvalConvert, php_class, php_const, php_extern, php_function, php_impl, php_interface,
         php_module, wrap_constant, wrap_function, zend_fastcall,