Add sapi zts test (#496)

Co-authored-by: Pierre Tondereau <[email protected]>

Author: Qard

.github/workflows/build.yml

@@ -169,8 +169,11 @@ jobs:
         if: "!(contains(matrix.os, 'macos') && matrix.rust == 'nightly')"
         run: cargo test --release --workspace --features closure,anyhow,runtime --no-fail-fast
   test-embed:
-    name: Test with embed
+    name: Test with embed (${{ matrix.phpts }})
     runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        phpts: [ts, nts]
     env:
       clang: "17"
       php_version: "8.5"
@@ -183,6 +186,7 @@ jobs:
         with:
           php-version: ${{ env.php_version }}
         env:
+          phpts: ${{ matrix.phpts }}
           debug: true
 
       - name: Setup Rust

crates/macros/src/module.rs

@@ -35,11 +35,16 @@ pub fn parser(input: ItemFn) -> Result<TokenStream> {
                 let b = __EXT_PHP_RS_MODULE_STARTUP
                     .lock()
                     .take()
-                    .inspect(|_| ::ext_php_rs::internal::ext_php_rs_startup())
-                    .expect("Module startup function has already been called.")
-                    .startup(ty, mod_num)
-                    .map(|_| 0)
-                    .unwrap_or(1);
+                    .map(|startup| {
+                        ::ext_php_rs::internal::ext_php_rs_startup();
+                        startup.startup(ty, mod_num).map(|_| 0).unwrap_or(1)
+                    })
+                    .unwrap_or_else(|| {
+                        // Module already started, call ext_php_rs_startup for idempotent
+                        // initialization (e.g., Closure::build early-returns if already built)
+                        ::ext_php_rs::internal::ext_php_rs_startup();
+                        0
+                    });
                 a | b
             }
 

src/builders/module.rs

@@ -375,38 +375,63 @@ impl TryFrom<ModuleBuilder<'_>> for (ModuleEntry, ModuleStartup) {
             enums: builder.enums,
         };
 
-        Ok((
-            ModuleEntry {
-                size: mem::size_of::<ModuleEntry>().try_into()?,
-                zend_api: ZEND_MODULE_API_NO,
-                zend_debug: u8::from(PHP_DEBUG),
-                zts: u8::from(PHP_ZTS),
-                ini_entry: ptr::null(),
-                deps: ptr::null(),
-                name,
-                functions,
-                module_startup_func: builder.startup_func,
-                module_shutdown_func: builder.shutdown_func,
-                request_startup_func: builder.request_startup_func,
-                request_shutdown_func: builder.request_shutdown_func,
-                info_func: builder.info_func,
-                version,
-                globals_size: 0,
-                #[cfg(not(php_zts))]
-                globals_ptr: ptr::null_mut(),
-                #[cfg(php_zts)]
-                globals_id_ptr: ptr::null_mut(),
-                globals_ctor: None,
-                globals_dtor: None,
-                post_deactivate_func: builder.post_deactivate_func,
-                module_started: 0,
-                type_: 0,
-                handle: ptr::null_mut(),
-                module_number: 0,
-                build_id: unsafe { ext_php_rs_php_build_id() },
-            },
-            startup,
-        ))
+        #[cfg(not(php_zts))]
+        let module_entry = ModuleEntry {
+            size: mem::size_of::<ModuleEntry>().try_into()?,
+            zend_api: ZEND_MODULE_API_NO,
+            zend_debug: u8::from(PHP_DEBUG),
+            zts: u8::from(PHP_ZTS),
+            ini_entry: ptr::null(),
+            deps: ptr::null(),
+            name,
+            functions,
+            module_startup_func: builder.startup_func,
+            module_shutdown_func: builder.shutdown_func,
+            request_startup_func: builder.request_startup_func,
+            request_shutdown_func: builder.request_shutdown_func,
+            info_func: builder.info_func,
+            version,
+            globals_size: 0,
+            globals_ptr: ptr::null_mut(),
+            globals_ctor: None,
+            globals_dtor: None,
+            post_deactivate_func: builder.post_deactivate_func,
+            module_started: 0,
+            type_: 0,
+            handle: ptr::null_mut(),
+            module_number: 0,
+            build_id: unsafe { ext_php_rs_php_build_id() },
+        };
+
+        #[cfg(php_zts)]
+        let module_entry = ModuleEntry {
+            size: mem::size_of::<ModuleEntry>().try_into()?,
+            zend_api: ZEND_MODULE_API_NO,
+            zend_debug: u8::from(PHP_DEBUG),
+            zts: u8::from(PHP_ZTS),
+            ini_entry: ptr::null(),
+            deps: ptr::null(),
+            name,
+            functions,
+            module_startup_func: builder.startup_func,
+            module_shutdown_func: builder.shutdown_func,
+            request_startup_func: builder.request_startup_func,
+            request_shutdown_func: builder.request_shutdown_func,
+            info_func: builder.info_func,
+            version,
+            globals_size: 0,
+            globals_id_ptr: ptr::null_mut(),
+            globals_ctor: None,
+            globals_dtor: None,
+            post_deactivate_func: builder.post_deactivate_func,
+            module_started: 0,
+            type_: 0,
+            handle: ptr::null_mut(),
+            module_number: 0,
+            build_id: unsafe { ext_php_rs_php_build_id() },
+        };
+
+        Ok((module_entry, startup))
     }
 }
 

src/closure.rs

@@ -115,11 +115,16 @@ impl Closure {
     /// function should only be called once inside your module startup
     /// function.
     ///
+    /// If the class has already been built, this function returns early without
+    /// doing anything. This allows for safe repeated calls in test environments.
+    ///
     /// # Panics
     ///
-    /// Panics if the function is called more than once.
+    /// Panics if the `RustClosure` PHP class cannot be registered.
     pub fn build() {
-        assert!(!CLOSURE_META.has_ce(), "Closure class already built.");
+        if CLOSURE_META.has_ce() {
+            return;
+        }
 
         ClassBuilder::new("RustClosure")
             .method(

src/embed/embed.c

@@ -44,6 +44,12 @@ SAPI_API void ext_php_rs_sapi_per_thread_init() {
   #endif
 }
 
+SAPI_API void ext_php_rs_sapi_per_thread_shutdown() {
+  #ifdef ZTS
+    ts_free_thread();
+  #endif
+}
+
 void ext_php_rs_php_error(int type, const char *format, ...) {
   va_list args;
   va_start(args, format);

src/embed/embed.h

@@ -9,5 +9,6 @@ void* ext_php_rs_embed_callback(int argc, char** argv, void* (*callback)(void *)
 void ext_php_rs_sapi_startup();
 void ext_php_rs_sapi_shutdown();
 void ext_php_rs_sapi_per_thread_init();
+void ext_php_rs_sapi_per_thread_shutdown();
 
 void ext_php_rs_php_error(int type, const char *format, ...);

src/embed/ffi.rs

@@ -20,6 +20,7 @@ unsafe extern "C" {
     pub fn ext_php_rs_sapi_startup();
     pub fn ext_php_rs_sapi_shutdown();
     pub fn ext_php_rs_sapi_per_thread_init();
+    pub fn ext_php_rs_sapi_per_thread_shutdown();
 
     pub fn ext_php_rs_php_error(
         type_: ::std::os::raw::c_int,

tests/sapi.rs

@@ -9,17 +9,30 @@
 extern crate ext_php_rs;
 
 use ext_php_rs::builders::SapiBuilder;
-use ext_php_rs::embed::{Embed, ext_php_rs_sapi_startup};
+use ext_php_rs::embed::{Embed, ext_php_rs_sapi_shutdown, ext_php_rs_sapi_startup};
 use ext_php_rs::ffi::{
     ZEND_RESULT_CODE_SUCCESS, php_module_shutdown, php_module_startup, php_request_shutdown,
     php_request_startup, sapi_shutdown, sapi_startup,
 };
 use ext_php_rs::prelude::*;
 use ext_php_rs::zend::try_catch_first;
 use std::ffi::c_char;
+use std::sync::Mutex;
+
+#[cfg(php_zts)]
+use ext_php_rs::embed::{ext_php_rs_sapi_per_thread_init, ext_php_rs_sapi_per_thread_shutdown};
+#[cfg(php_zts)]
+use std::sync::Arc;
+#[cfg(php_zts)]
+use std::thread;
 
 static mut LAST_OUTPUT: String = String::new();
 
+// Global mutex to ensure SAPI tests don't run concurrently. PHP does not allow
+// multiple SAPIs to exist at the same time. This prevents the tests from
+// overwriting each other's state.
+static SAPI_TEST_MUTEX: Mutex<()> = Mutex::new(());
+
 extern "C" fn output_tester(str: *const c_char, str_length: usize) -> usize {
     let char = unsafe { std::slice::from_raw_parts(str.cast::<u8>(), str_length) };
     let string = String::from_utf8_lossy(char);
@@ -35,6 +48,8 @@ extern "C" fn output_tester(str: *const c_char, str_length: usize) -> usize {
 
 #[test]
 fn test_sapi() {
+    let _guard = SAPI_TEST_MUTEX.lock().unwrap();
+
     let mut builder = SapiBuilder::new("test", "Test");
     builder = builder.ub_write_function(output_tester);
 
@@ -86,6 +101,10 @@ fn test_sapi() {
     unsafe {
         sapi_shutdown();
     }
+
+    unsafe {
+        ext_php_rs_sapi_shutdown();
+    }
 }
 
 /// Gives you a nice greeting!
@@ -102,3 +121,96 @@ pub fn hello_world(name: String) -> String {
 pub fn module(module: ModuleBuilder) -> ModuleBuilder {
     module.function(wrap_function!(hello_world))
 }
+
+#[test]
+#[cfg(php_zts)]
+fn test_sapi_multithread() {
+    let _guard = SAPI_TEST_MUTEX.lock().unwrap();
+
+    let mut builder = SapiBuilder::new("test-mt", "Test Multi-threaded");
+    builder = builder.ub_write_function(output_tester);
+
+    let sapi = builder.build().unwrap().into_raw();
+    let module = get_module();
+
+    unsafe {
+        ext_php_rs_sapi_startup();
+    }
+
+    unsafe {
+        sapi_startup(sapi);
+    }
+
+    unsafe {
+        php_module_startup(sapi, module);
+    }
+
+    let results = Arc::new(Mutex::new(Vec::new()));
+    let mut handles = vec![];
+
+    for i in 0..4 {
+        let results = Arc::clone(&results);
+
+        let handle = thread::spawn(move || {
+            unsafe {
+                ext_php_rs_sapi_per_thread_init();
+            }
+
+            let result = unsafe { php_request_startup() };
+            assert_eq!(result, ZEND_RESULT_CODE_SUCCESS);
+
+            let _ = try_catch_first(|| {
+                let eval_result = Embed::eval(&format!("hello_world('thread-{i}');"));
+
+                match eval_result {
+                    Ok(zval) => {
+                        assert!(zval.is_string());
+                        let string = zval.string().unwrap();
+                        let output = string.to_string();
+                        assert_eq!(output, format!("Hello, thread-{i}!"));
+
+                        results.lock().unwrap().push((i, output));
+                    }
+                    Err(e) => panic!("Evaluation failed in thread {i}: {e:?}"),
+                }
+            });
+
+            unsafe {
+                php_request_shutdown(std::ptr::null_mut());
+            }
+
+            unsafe {
+                ext_php_rs_sapi_per_thread_shutdown();
+            }
+        });
+
+        handles.push(handle);
+    }
+
+    for handle in handles {
+        handle.join().expect("Thread panicked");
+    }
+
+    let results = results.lock().unwrap();
+    assert_eq!(results.len(), 4);
+
+    for i in 0..4 {
+        assert!(
+            results
+                .iter()
+                .any(|(idx, output)| { *idx == i && output == &format!("Hello, thread-{i}!") })
+        );
+    }
+
+    unsafe {
+        php_module_shutdown();
+    }
+
+    unsafe {
+        sapi_shutdown();
+    }
+
+    unsafe {
+        ext_php_rs_sapi_shutdown();
+    }
+}