Skip to content

Commit d1428af

Browse files
Qardclaude
andcommitted
Simplify per-thread PHP init - ThreadScope only, remove Sapi::startup()
The previous approach of calling php_module_startup() per-request caused assertion failures in zend_new_interned_string_permanent when called concurrently from multiple threads. PHP's architecture uses: - php_module_startup() once during SAPI init (global state, interned strings) - ts_resource(0) per-thread to initialize TSRM thread-local storage The fix: - Keep php_module_startup() as one-time call in Sapi::new() on main thread - ThreadScope calls ext_php_rs_sapi_per_thread_init() which calls ts_resource(0) - Remove Sapi::startup() that was calling php_module_startup() per-request This properly separates global initialization from per-thread TLS setup. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent 2561124 commit d1428af

File tree

3 files changed

+4
-28
lines changed

3 files changed

+4
-28
lines changed

src/embed.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -341,16 +341,11 @@ impl Handler for Embed {
341341

342342
// Spawn blocking PHP execution - ALL PHP operations happen here
343343
let blocking_handle = tokio::task::spawn_blocking(move || {
344-
// Keep sapi alive for the duration of this task
345-
let sapi = _sapi;
344+
// Keep _sapi alive for the duration of the blocking task
345+
let _ = &_sapi;
346+
346347
// Initialize thread-local storage for this worker thread
347348
let _thread_scope = ThreadScope::new();
348-
// Call SAPI startup on THIS thread to initialize PHP's memory allocator.
349-
// This is required before any estrdup calls can be made.
350-
// php_module_startup() sets up per-thread state needed for emalloc/estrdup.
351-
sapi.startup().map_err(|_| EmbedRequestError::SapiNotStarted)?;
352-
// Keep sapi alive for the duration of this task
353-
let _sapi = sapi;
354349

355350
// Setup RequestContext (always streaming from SAPI perspective)
356351
// RequestContext::new() will extract the request body's read stream and add it as RequestStream extension

src/sapi.rs

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -114,24 +114,6 @@ impl Sapi {
114114
})
115115
}
116116

117-
/// Initialize PHP for the current thread and call the SAPI module startup.
118-
/// This must be called on each worker thread before using PHP's memory allocator (estrdup).
119-
/// On ZTS builds, this initializes thread-local storage and module state.
120-
pub fn startup(&self) -> Result<(), EmbedRequestError> {
121-
let sapi = &mut self
122-
.module
123-
.write()
124-
.map_err(|_| EmbedRequestError::SapiNotStarted)?;
125-
126-
if let Some(startup) = sapi.startup {
127-
if unsafe { startup(sapi.as_mut()) } != ZEND_RESULT_CODE_SUCCESS {
128-
return Err(EmbedRequestError::SapiNotStarted);
129-
}
130-
}
131-
132-
Ok(())
133-
}
134-
135117
pub fn shutdown(&self) -> Result<(), EmbedRequestError> {
136118
// Only shutdown if we're on the same thread that created this Sapi
137119
let current_thread = std::thread::current().id();

src/scopes.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@ use ext_php_rs::{
99
embed::{ext_php_rs_sapi_per_thread_init, ext_php_rs_sapi_per_thread_shutdown},
1010
ffi::{
1111
_zend_file_handle__bindgen_ty_1, php_request_shutdown, php_request_startup,
12-
zend_destroy_file_handle, zend_file_handle, zend_stream_init_filename,
13-
ZEND_RESULT_CODE_SUCCESS,
12+
zend_destroy_file_handle, zend_file_handle, zend_stream_init_filename, ZEND_RESULT_CODE_SUCCESS,
1413
},
1514
};
1615

0 commit comments

Comments
 (0)