Skip to content

Commit 2561124

Browse files
Qardclaude
andcommitted
Fix estrdup crash by calling sapi.startup() inside spawn_blocking
The crash in CI occurred because estrdup was being called before PHP's memory allocator was properly initialized on the worker thread. The root cause was identified through debug logging that showed the crash happened at the first estrdup call after ThreadScope::new(). The fix adds a sapi.startup() call inside spawn_blocking, after ThreadScope::new() but before any estrdup calls. This mirrors the main branch behavior where self.sapi.startup() was called in every handle() before estrdup operations. Key changes: - Added Sapi::startup() method that calls the SAPI module startup function - Call sapi.startup() inside spawn_blocking to initialize PHP's memory allocator on the worker thread - Removed debug logging that was added to diagnose the crash The SAPI startup (php_module_startup) sets up per-thread state needed for emalloc/estrdup to work. Without it, ts_resource(0) alone doesn't fully initialize the memory allocator on ZTS builds. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent 40761fc commit 2561124

File tree

3 files changed

+36
-46
lines changed

3 files changed

+36
-46
lines changed

src/embed.rs

Lines changed: 18 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -341,67 +341,47 @@ 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] spawn_blocking: starting");
345344
// Keep sapi alive for the duration of this task
346-
let _sapi = _sapi;
347-
eprintln!("[DEBUG] spawn_blocking: creating ThreadScope");
345+
let sapi = _sapi;
346+
// Initialize thread-local storage for this worker thread
348347
let _thread_scope = ThreadScope::new();
349-
eprintln!("[DEBUG] spawn_blocking: ThreadScope created");
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;
350354

351355
// Setup RequestContext (always streaming from SAPI perspective)
352356
// RequestContext::new() will extract the request body's read stream and add it as RequestStream extension
353-
eprintln!("[DEBUG] spawn_blocking: creating RequestContext");
354357
let ctx = RequestContext::new(
355358
request,
356359
docroot.clone(),
357360
response_writer.clone(),
358361
headers_sent_tx,
359362
);
360-
eprintln!("[DEBUG] spawn_blocking: setting RequestContext::current");
361363
RequestContext::set_current(Box::new(ctx));
362-
eprintln!("[DEBUG] spawn_blocking: RequestContext set");
363364

364-
// All estrdup calls MUST happen here, inside spawn_blocking, after ThreadScope::new()
365-
// has initialized PHP's memory manager for this thread.
366-
// These will be freed by efree in sapi_module_deactivate during request shutdown.
367-
eprintln!("[DEBUG] estrdup 1/5: request_uri_str - BEFORE");
365+
// All estrdup calls happen here, inside spawn_blocking, after ThreadScope::new()
366+
// has initialized PHP's thread-local storage. These will be freed by efree in
367+
// sapi_module_deactivate during request shutdown.
368368
let request_uri_c = estrdup(request_uri_str.as_str());
369-
eprintln!("[DEBUG] estrdup 1/5: request_uri_str - AFTER (ptr={:?})", request_uri_c);
370-
371-
eprintln!("[DEBUG] estrdup 2/5: path_translated - BEFORE");
372369
let path_translated = estrdup(translated_path_str.as_str());
373-
eprintln!("[DEBUG] estrdup 2/5: path_translated - AFTER (ptr={:?})", path_translated);
374-
375-
eprintln!("[DEBUG] estrdup 3/5: request_method - BEFORE");
376370
let request_method = estrdup(method_str.as_str());
377-
eprintln!("[DEBUG] estrdup 3/5: request_method - AFTER (ptr={:?})", request_method);
378-
379-
eprintln!("[DEBUG] estrdup 4/5: query_string - BEFORE");
380371
let query_string = estrdup(query_str.as_str());
381-
eprintln!("[DEBUG] estrdup 4/5: query_string - AFTER (ptr={:?})", query_string);
382-
383-
eprintln!("[DEBUG] estrdup 5/5: content_type - BEFORE");
384372
let content_type = content_type_str
385373
.as_ref()
386-
.map(|s| {
387-
eprintln!("[DEBUG] estrdup 5/5: content_type has value, calling estrdup");
388-
estrdup(s.as_str())
389-
})
374+
.map(|s| estrdup(s.as_str()))
390375
.unwrap_or(std::ptr::null_mut());
391-
eprintln!("[DEBUG] estrdup 5/5: content_type - AFTER (ptr={:?})", content_type);
392376

393377
// Prepare argv pointers
394378
let argc = args.len() as i32;
395-
eprintln!("[DEBUG] estrdup argv: {} args to process - BEFORE", argc);
396-
let mut argv_ptrs: Vec<*mut c_char> = args.iter().enumerate().map(|(i, s)| {
397-
eprintln!("[DEBUG] estrdup argv[{}] - BEFORE", i);
398-
let ptr = estrdup(s.as_str());
399-
eprintln!("[DEBUG] estrdup argv[{}] - AFTER (ptr={:?})", i, ptr);
400-
ptr
401-
}).collect();
402-
eprintln!("[DEBUG] estrdup argv complete");
403-
404-
// Set SAPI globals
379+
let mut argv_ptrs: Vec<*mut c_char> = args
380+
.iter()
381+
.map(|s| estrdup(s.as_str()))
382+
.collect();
383+
384+
// Set SAPI globals BEFORE php_request_startup since PHP reads these during initialization
405385
{
406386
let mut globals = SapiGlobals::get_mut();
407387
globals.options |= ext_php_rs::ffi::SAPI_OPTION_NO_CHDIR as i32;

src/sapi.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,24 @@ 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+
117135
pub fn shutdown(&self) -> Result<(), EmbedRequestError> {
118136
// Only shutdown if we're on the same thread that created this Sapi
119137
let current_thread = std::thread::current().id();

src/scopes.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,24 +20,18 @@ pub(crate) struct ThreadScope();
2020

2121
impl ThreadScope {
2222
pub fn new() -> Self {
23-
let thread_id = std::thread::current().id();
24-
eprintln!("[DEBUG] ThreadScope::new() on thread {:?} - calling ext_php_rs_sapi_per_thread_init", thread_id);
2523
unsafe {
2624
ext_php_rs_sapi_per_thread_init();
2725
}
28-
eprintln!("[DEBUG] ThreadScope::new() on thread {:?} - ext_php_rs_sapi_per_thread_init complete", thread_id);
2926
Self()
3027
}
3128
}
3229

3330
impl Drop for ThreadScope {
3431
fn drop(&mut self) {
35-
let thread_id = std::thread::current().id();
36-
eprintln!("[DEBUG] ThreadScope::drop() on thread {:?} - calling ext_php_rs_sapi_per_thread_shutdown", thread_id);
3732
unsafe {
3833
ext_php_rs_sapi_per_thread_shutdown();
3934
}
40-
eprintln!("[DEBUG] ThreadScope::drop() on thread {:?} - ext_php_rs_sapi_per_thread_shutdown complete", thread_id);
4135
}
4236
}
4337

@@ -73,7 +67,6 @@ impl FileHandleScope {
7367
P: AsRef<Path>,
7468
{
7569
let path_str = path.as_ref().to_str().unwrap();
76-
eprintln!("[DEBUG] FileHandleScope::new - estrdup for path '{}' - BEFORE", path_str);
7770
let mut handle = zend_file_handle {
7871
handle: _zend_file_handle__bindgen_ty_1 {
7972
fp: std::ptr::null_mut(),
@@ -88,7 +81,6 @@ impl FileHandleScope {
8881
};
8982

9083
let path = estrdup(path_str);
91-
eprintln!("[DEBUG] FileHandleScope::new - estrdup for path - AFTER (ptr={:?})", path);
9284

9385
unsafe {
9486
zend_stream_init_filename(&mut handle, path);

0 commit comments

Comments
 (0)