Skip to content

Commit 858d729

Browse files
Qardclaude
andcommitted
Fix ZTS heap corruption by calling php_module_startup only once
The previous code called php_module_startup() per-thread via Sapi::startup(), but this function is only meant to be called once during application initialization. Calling it multiple times caused heap corruption in PHP's memory allocator, manifesting as EXC_BAD_ACCESS (code=2) crashes in _estrdup on macOS aarch64. The correct ZTS lifecycle is: 1. php_module_startup() - once at app init (in Sapi::new()) 2. ts_resource(0) - per-thread TLS init (in ThreadScope::new()) 3. php_request_startup()/shutdown() - per-request (in RequestScope) Sapi::startup() now only verifies the SAPI is accessible (read lock) rather than reinitializing PHP modules. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent eac0104 commit 858d729

File tree

2 files changed

+19
-23
lines changed

2 files changed

+19
-23
lines changed

src/embed.rs

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -341,17 +341,16 @@ 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-
eprintln!("[DEBUG 1] spawn_blocking started");
345-
// Initialize thread-local storage for this worker thread
344+
// Initialize thread-local storage for this worker thread.
345+
// This calls ext_php_rs_sapi_per_thread_init() -> ts_resource(0) which sets up
346+
// PHP's thread-local storage for the current thread.
346347
let _thread_scope = ThreadScope::new();
347-
eprintln!("[DEBUG 2] ThreadScope::new() completed");
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-
// The startup call is serialized via write lock to prevent concurrent php_module_startup.
348+
349+
// Verify SAPI is accessible (no longer calls php_module_startup per-thread).
350+
// php_module_startup is only called once in Sapi::new(), not per-thread.
351351
_sapi
352352
.startup()
353353
.map_err(|_| EmbedRequestError::SapiNotStarted)?;
354-
eprintln!("[DEBUG 3] sapi.startup() completed");
355354

356355
// Setup RequestContext (always streaming from SAPI perspective)
357356
// RequestContext::new() will extract the request body's read stream and add it as RequestStream extension
@@ -361,16 +360,12 @@ impl Handler for Embed {
361360
response_writer.clone(),
362361
headers_sent_tx,
363362
);
364-
eprintln!("[DEBUG 4] RequestContext::new() completed");
365363
RequestContext::set_current(Box::new(ctx));
366-
eprintln!("[DEBUG 5] RequestContext::set_current() completed");
367364

368365
// All estrdup calls happen here, inside spawn_blocking, after ThreadScope::new()
369366
// has initialized PHP's thread-local storage. These will be freed by efree in
370367
// sapi_module_deactivate during request shutdown.
371-
eprintln!("[DEBUG 6] About to call estrdup for request_uri_str: {:?}", request_uri_str);
372368
let request_uri_c = estrdup(request_uri_str.as_str());
373-
eprintln!("[DEBUG 7] estrdup(request_uri_str) completed");
374369
let path_translated = estrdup(translated_path_str.as_str());
375370
let request_method = estrdup(method_str.as_str());
376371
let query_string = estrdup(query_str.as_str());

src/sapi.rs

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -114,22 +114,23 @@ 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-
/// Uses a write lock to serialize startup calls across threads.
117+
/// Verify that the SAPI module has been initialized.
118+
///
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.
122+
///
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.
121125
pub fn startup(&self) -> Result<(), EmbedRequestError> {
122-
let sapi = &mut self
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
123130
.module
124-
.write()
131+
.read()
125132
.map_err(|_| EmbedRequestError::SapiNotStarted)?;
126133

127-
if let Some(startup) = sapi.startup {
128-
if unsafe { startup(sapi.as_mut()) } != ZEND_RESULT_CODE_SUCCESS {
129-
return Err(EmbedRequestError::SapiNotStarted);
130-
}
131-
}
132-
133134
Ok(())
134135
}
135136

0 commit comments

Comments
 (0)