From 30ecc9f95228c09707366ffc0f34703231fa873b Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Mon, 7 Jul 2025 17:32:27 +0800 Subject: [PATCH 1/7] test: add basic multithreaded SAPI test --- tests/sapi.rs | 105 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 104 insertions(+), 1 deletion(-) diff --git a/tests/sapi.rs b/tests/sapi.rs index 11c67d7f53..1951504de8 100644 --- a/tests/sapi.rs +++ b/tests/sapi.rs @@ -9,7 +9,9 @@ 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_per_thread_init, 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, @@ -17,9 +19,16 @@ use ext_php_rs::ffi::{ use ext_php_rs::prelude::*; use ext_php_rs::zend::try_catch_first; use std::ffi::c_char; +use std::sync::{Arc, Mutex}; +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::(), str_length) }; let string = String::from_utf8_lossy(char); @@ -35,6 +44,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 +97,10 @@ fn test_sapi() { unsafe { sapi_shutdown(); } + + unsafe { + ext_php_rs_sapi_shutdown(); + } } /// Gives you a nice greeting! @@ -102,3 +117,91 @@ pub fn hello_world(name: String) -> String { pub fn module(module: ModuleBuilder) -> ModuleBuilder { module.function(wrap_function!(hello_world)) } + +#[test] +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()); + } + }); + + 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(); + } +} From 40555e81f16cab70405ebfb6be3b2885e7de5719 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Wed, 29 Oct 2025 14:39:07 -0700 Subject: [PATCH 2/7] fix(closure): early-exit rather than assert if already built --- src/closure.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/closure.rs b/src/closure.rs index 10d4ab2279..f184ff8f4d 100644 --- a/src/closure.rs +++ b/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( From e50789e7a9db27c17cb17a30d369cabdd623e5b3 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Wed, 29 Oct 2025 15:49:54 -0700 Subject: [PATCH 3/7] fix(macros): make get_module() startup idempotent --- crates/macros/src/module.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/macros/src/module.rs b/crates/macros/src/module.rs index 2318424aa9..46115b26a9 100644 --- a/crates/macros/src/module.rs +++ b/crates/macros/src/module.rs @@ -35,11 +35,16 @@ pub fn parser(input: ItemFn) -> Result { 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 } From 44b188b94a15385a81fb4980a194755032b6b682 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Mon, 3 Nov 2025 15:26:28 -0800 Subject: [PATCH 4/7] test: only run multi-threaded sapi test when ZTS enabled Co-authored-by: Pierre Tondereau --- tests/sapi.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/sapi.rs b/tests/sapi.rs index 1951504de8..639505ef2a 100644 --- a/tests/sapi.rs +++ b/tests/sapi.rs @@ -9,9 +9,7 @@ extern crate ext_php_rs; use ext_php_rs::builders::SapiBuilder; -use ext_php_rs::embed::{ - Embed, ext_php_rs_sapi_per_thread_init, ext_php_rs_sapi_shutdown, 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, @@ -19,7 +17,13 @@ use ext_php_rs::ffi::{ use ext_php_rs::prelude::*; use ext_php_rs::zend::try_catch_first; use std::ffi::c_char; -use std::sync::{Arc, Mutex}; +use std::sync::Mutex; + +#[cfg(php_zts)] +use ext_php_rs::embed::ext_php_rs_sapi_per_thread_init; +#[cfg(php_zts)] +use std::sync::Arc; +#[cfg(php_zts)] use std::thread; static mut LAST_OUTPUT: String = String::new(); @@ -119,6 +123,7 @@ pub fn module(module: ModuleBuilder) -> ModuleBuilder { } #[test] +#[cfg(php_zts)] fn test_sapi_multithread() { let _guard = SAPI_TEST_MUTEX.lock().unwrap(); From 456c9ee9e61950e011676cea858f4ede69fa815d Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Mon, 10 Nov 2025 13:14:10 -0800 Subject: [PATCH 5/7] test: add phpts matrix build --- .github/workflows/build.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 91c42c6e41..205b775efa 100644 --- a/.github/workflows/build.yml +++ b/.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 From ae8e07676ed67d36aef22ddcf7b3fb17bb01562b Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Mon, 10 Nov 2025 13:30:57 -0800 Subject: [PATCH 6/7] fix(builders): module entries must switch globals ptr property by zts setting --- src/builders/module.rs | 89 +++++++++++++++++++++++++++--------------- 1 file changed, 57 insertions(+), 32 deletions(-) diff --git a/src/builders/module.rs b/src/builders/module.rs index 3f8e8f8fb5..0d334d0d69 100644 --- a/src/builders/module.rs +++ b/src/builders/module.rs @@ -375,38 +375,63 @@ impl TryFrom> for (ModuleEntry, ModuleStartup) { enums: builder.enums, }; - Ok(( - ModuleEntry { - size: mem::size_of::().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::().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::().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)) } } From c01793e17de17bd4dc3f55706db779e0cf9281dc Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Tue, 11 Nov 2025 15:05:08 -0800 Subject: [PATCH 7/7] fix: add ext_php_rs_sapi_per_thread_shutdown --- src/embed/embed.c | 6 ++++++ src/embed/embed.h | 1 + src/embed/ffi.rs | 1 + tests/sapi.rs | 6 +++++- 4 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/embed/embed.c b/src/embed/embed.c index d4b1608cbc..4af1c172a2 100644 --- a/src/embed/embed.c +++ b/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); diff --git a/src/embed/embed.h b/src/embed/embed.h index ae6b7c3826..be6c365725 100644 --- a/src/embed/embed.h +++ b/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, ...); diff --git a/src/embed/ffi.rs b/src/embed/ffi.rs index 7749ea051d..c4a9fffc29 100644 --- a/src/embed/ffi.rs +++ b/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, diff --git a/tests/sapi.rs b/tests/sapi.rs index 639505ef2a..73136d4f6f 100644 --- a/tests/sapi.rs +++ b/tests/sapi.rs @@ -20,7 +20,7 @@ use std::ffi::c_char; use std::sync::Mutex; #[cfg(php_zts)] -use ext_php_rs::embed::ext_php_rs_sapi_per_thread_init; +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)] @@ -178,6 +178,10 @@ fn test_sapi_multithread() { unsafe { php_request_shutdown(std::ptr::null_mut()); } + + unsafe { + ext_php_rs_sapi_per_thread_shutdown(); + } }); handles.push(handle);