Skip to content

Commit 3890cf8

Browse files
Qardclaude
andcommitted
Add ModuleScope for per-thread php_module_startup in ZTS mode
In PHP's ZTS (Thread Safe) mode, php_module_startup must be called per-thread after ts_resource(0) has initialized the thread's local storage. This commit adds: 1. ModuleScope type that calls php_module_startup per-thread 2. Sapi::module_scope() method to create ModuleScope with proper pointers 3. Per-thread module initialization in spawn_blocking tasks Key insight: php_module_startup is called once in Sapi::new() for the main thread, and then again per worker thread via module_scope(). php_module_shutdown should only be called once during SAPI shutdown, NOT per-thread (per-thread cleanup is handled by ts_free_thread). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent 982e0c5 commit 3890cf8

File tree

3 files changed

+57
-31
lines changed

3 files changed

+57
-31
lines changed

src/embed.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -349,12 +349,13 @@ impl Handler for Embed {
349349
let _thread_scope = ThreadScope::new();
350350
eprintln!("DEBUG [spawn_blocking] Step 2: ThreadScope created");
351351

352-
// Verify SAPI is accessible (no longer calls php_module_startup per-thread).
353-
// php_module_startup is only called once in Sapi::new(), not per-thread.
354-
sapi
355-
.startup()
352+
// Initialize PHP module for this thread.
353+
// In ZTS mode, php_module_startup must be called per-thread after the thread's
354+
// local storage has been initialized. ModuleScope ensures matching shutdown.
355+
let _module_scope = sapi
356+
.module_scope()
356357
.map_err(|_| EmbedRequestError::SapiNotStarted)?;
357-
eprintln!("DEBUG [spawn_blocking] Step 3: SAPI startup verified");
358+
eprintln!("DEBUG [spawn_blocking] Step 3: ModuleScope created (php_module_startup called)");
358359

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

src/sapi.rs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ impl Sapi {
8686
ext_php_rs_sapi_startup();
8787
sapi_startup(boxed.as_mut());
8888

89+
// Call module startup for the main thread that's constructing this Sapi.
90+
// Each worker thread also needs its own module startup via module_scope().
8991
if let Some(startup) = boxed.startup {
9092
startup(boxed.as_mut());
9193
}
@@ -114,24 +116,22 @@ impl Sapi {
114116
})
115117
}
116118

117-
/// Verify that the SAPI module has been initialized.
119+
/// Creates a new ModuleScope for per-thread module startup.
118120
///
119-
/// In ZTS mode, thread-local storage is initialized by ThreadScope::new() which
120-
/// calls ext_php_rs_sapi_per_thread_init() -> ts_resource(0). The module startup
121-
/// (php_module_startup) is only called once in Sapi::new(), not per-thread.
121+
/// In ZTS mode, php_module_startup must be called per-thread after the thread's
122+
/// local storage has been initialized via ts_resource(0) in ThreadScope::new().
122123
///
123-
/// This method verifies the SAPI is accessible, but no longer calls php_module_startup
124-
/// per-thread as that causes heap corruption when called multiple times.
125-
pub fn startup(&self) -> Result<(), EmbedRequestError> {
126-
// Just verify we can acquire a read lock - the actual module startup
127-
// already happened in Sapi::new(). ThreadScope::new() handles per-thread
128-
// TLS initialization via ts_resource(0).
129-
let _sapi = self
124+
/// This method provides access to the SAPI module and module entry pointers
125+
/// needed by ModuleScope::new().
126+
pub fn module_scope(&self) -> Result<crate::scopes::ModuleScope, EmbedRequestError> {
127+
let mut sapi = self
130128
.module
131-
.read()
129+
.write()
132130
.map_err(|_| EmbedRequestError::SapiNotStarted)?;
133131

134-
Ok(())
132+
unsafe {
133+
crate::scopes::ModuleScope::new(sapi.as_mut() as *mut _, get_module())
134+
}
135135
}
136136

137137
pub fn shutdown(&self) -> Result<(), EmbedRequestError> {

src/scopes.rs

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,11 @@ use std::{
66

77
use ext_php_rs::{
88
alloc::{efree, estrdup},
9-
embed::{ext_php_rs_sapi_per_thread_init, ext_php_rs_sapi_per_thread_shutdown},
9+
embed::{ext_php_rs_sapi_per_thread_init, ext_php_rs_sapi_per_thread_shutdown, SapiModule},
1010
ffi::{
11-
_zend_file_handle__bindgen_ty_1, php_module_shutdown, php_request_shutdown, php_request_startup,
12-
zend_destroy_file_handle, zend_file_handle, zend_stream_init_filename, ZEND_RESULT_CODE_SUCCESS,
11+
_zend_file_handle__bindgen_ty_1, php_module_startup, php_request_shutdown, php_request_startup,
12+
zend_destroy_file_handle, zend_file_handle, zend_module_entry, zend_stream_init_filename,
13+
ZEND_RESULT_CODE_SUCCESS,
1314
},
1415
};
1516

@@ -38,21 +39,45 @@ impl Drop for ThreadScope {
3839
}
3940
}
4041

41-
/// A scope for PHP module shutdown (module startup is called once in Sapi::new()).
42-
/// This is used to ensure php_module_shutdown is called on drop.
42+
/// A scope for PHP module startup per-thread.
43+
/// This ensures that php_module_startup is called on scope creation.
4344
///
44-
/// NOTE: php_module_startup is only called once in Sapi::new() via the startup callback.
45-
/// Per-thread TLS initialization is handled by ThreadScope via ts_resource(0).
46-
#[allow(dead_code)]
45+
/// In ZTS mode, php_module_startup must be called per-thread after the thread's
46+
/// local storage has been initialized via ts_resource(0).
47+
///
48+
/// NOTE: php_module_shutdown should only be called once when the SAPI shuts down,
49+
/// NOT per-thread. Per-thread cleanup is handled by ThreadScope (ts_free_thread).
4750
pub(crate) struct ModuleScope;
4851

52+
impl ModuleScope {
53+
/// Creates a new ModuleScope, calling php_module_startup.
54+
///
55+
/// # Safety
56+
/// The sapi_module and module_entry pointers must be valid.
57+
pub unsafe fn new(
58+
sapi_module: *mut SapiModule,
59+
module_entry: *mut zend_module_entry,
60+
) -> Result<Self, EmbedRequestError> {
61+
eprintln!("DEBUG [ModuleScope::new] Calling php_module_startup()");
62+
63+
let result = php_module_startup(sapi_module, module_entry);
64+
if result != ZEND_RESULT_CODE_SUCCESS {
65+
eprintln!("DEBUG [ModuleScope::new] php_module_startup() FAILED");
66+
return Err(EmbedRequestError::SapiNotStarted);
67+
}
68+
69+
eprintln!("DEBUG [ModuleScope::new] php_module_startup() succeeded");
70+
Ok(Self)
71+
}
72+
}
73+
4974
impl Drop for ModuleScope {
5075
fn drop(&mut self) {
51-
eprintln!("DEBUG [ModuleScope::drop] Calling php_module_shutdown()");
52-
unsafe {
53-
php_module_shutdown();
54-
}
55-
eprintln!("DEBUG [ModuleScope::drop] php_module_shutdown() completed");
76+
// NOTE: We intentionally do NOT call php_module_shutdown() here.
77+
// php_module_shutdown should only be called once when the entire SAPI shuts down,
78+
// not per-thread. Per-thread cleanup is handled by ThreadScope::drop which calls
79+
// ext_php_rs_sapi_per_thread_shutdown() -> ts_free_thread().
80+
eprintln!("DEBUG [ModuleScope::drop] ModuleScope dropped (no php_module_shutdown called)");
5681
}
5782
}
5883

0 commit comments

Comments
 (0)