Skip to content

Conversation

@Qard
Copy link
Member

@Qard Qard commented Mar 7, 2025

Still lots more to be done, but with this there's now an interface to transport all the data of http request and response into and out of the other VM. It's also done generically, so could be easily reused for other languages in the future, we'd just need to implement the Handler trait to make use of those data types for the transition between Node.js req/res types and whichever languages own request and response data types or interfaces.

With this I'm able to inject the various header-related superglobals into PHP, execute code, and receive headers and status code written out to the response, transporting all of them back to Node.js. The next major bit of work is getting the input and output stream working. The POST body forwarding looks to be straightforward. I'm not yet sure why I'm not getting output to ub_write though. 🤔

@Qard Qard added the enhancement New feature or request label Mar 7, 2025
Need to wire them up to something now...
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Qard Qard force-pushed the add-generic-lang-handler-interface branch 7 times, most recently from 11393d8 to 7bbd342 Compare March 13, 2025 10:22
@Qard Qard force-pushed the add-generic-lang-handler-interface branch from 7bbd342 to 6c844f9 Compare March 13, 2025 10:37
@Qard Qard force-pushed the add-generic-lang-handler-interface branch from 12f2dee to ff04c06 Compare March 13, 2025 15:38
@Qard Qard force-pushed the add-generic-lang-handler-interface branch 11 times, most recently from ad7f8e9 to 7729bd5 Compare March 14, 2025 08:53
@Qard Qard force-pushed the add-generic-lang-handler-interface branch 21 times, most recently from 8f04561 to a0c467b Compare March 19, 2025 12:17
@Qard Qard force-pushed the add-generic-lang-handler-interface branch from a0c467b to e7cf385 Compare March 19, 2025 12:35
@Qard Qard mentioned this pull request Mar 26, 2025
@Qard Qard force-pushed the main branch 4 times, most recently from 3f7ed68 to c667963 Compare May 8, 2025 17:26
@mcollina mcollina closed this May 23, 2025
Qard added a commit that referenced this pull request Dec 15, 2025
The root cause of the CI crashes was that Arc<Sapi> could be dropped
(and tsrm_shutdown() called) while a blocking task was still executing
PHP code. This happened because:

1. Embed holds Arc<Sapi> to keep PHP SAPI alive
2. spawn_blocking() creates a task that uses PHP functions
3. If Embed is dropped before the blocking task completes, Sapi refcount
   goes to 0, triggering Sapi::drop() which calls tsrm_shutdown()
4. The still-running blocking task tries to call estrdup(), but TSRM
   storage is already freed, causing EXC_BAD_ACCESS at address 0x0

The fix clones Arc<Sapi> into the blocking task closure, ensuring the
Sapi stays alive for the entire duration of PHP operations.

Stack trace that led to this fix:
  thread #24, name = 'tokio-runtime-worker', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: libphp.dylib`_emalloc + 44
    frame #1: libphp.dylib`_estrdup + 40
    frame #2: binding.node`BlockingTask<T>::poll + 144

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants