Skip to content

fix: refuse to start on a ZTS build with Zend signals - #2639

Merged
dunglas merged 1 commit into
php:mainfrom
nicolas-grekas:zend-signals-guard
Sep 7, 2026
Merged

fix: refuse to start on a ZTS build with Zend signals#2639
dunglas merged 1 commit into
php:mainfrom
nicolas-grekas:zend-signals-guard

Conversation

@nicolas-grekas

Copy link
Copy Markdown
Contributor

PHP enables Zend signals by default, docs/compile.md and the Dockerfiles pass --disable-zend-signals, and nothing checks it. Building PHP by hand without that flag gives a server that never finishes booting, with memory growing by hundreds of megabytes per second until the process is killed. No error, no log line, and the stacks only show PHP copying hash tables, so it reads like a slow machine.

What happens: FrankenPHP has its own SAPI and never calls zend_signal_startup(), which the cli and embed SAPIs of php-src do, so zend_signal_globals_id stays 0. When zend_register_standard_ini_entries() reaches zend.signal_check, OnUpdateBool resolves its address as ts_resource(*(int *)mh_arg2) + mh_arg1; ts_resource(0) returns the tsrm_tls_entry of the calling thread itself, so the bool write lands 16 bytes into it, on the thread_id field, and zeroes its low byte. TSRM never matches that thread again: ts_resource_ex() appends a new resource set and tail-recurses on every call, each level running executor_globals_ctor() and copying the constant, function and class tables. Watchpoint on the field, and the chain measured at 4013 entries before the process was killed.

This checks the flag PHP already reports through frankenphp_get_config() and fails Init() with an actionable error. Against a PHP built without the flag the tests now stop in 0.00s with that message instead of hanging; against a correctly built PHP the suite is unchanged.

Non-ZTS builds keep working: their ini entries address the globals directly, TSRM is not involved, so the check is limited to ZTS.

The alternative would be to call zend_signal_startup() like the embed SAPI does. That is not enough: with Zend signals actually running, PHP's handlers and the Go runtime's fight over the same signals and the test suite crashes, which is presumably why the flag is documented in the first place.

@withinboredom withinboredom left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice find!

Comment thread docs/compile.md Outdated
Comment thread frankenphp.go Outdated
FrankenPHP has its own SAPI and never calls zend_signal_startup(), which
the cli and embed SAPIs of php-src do, so zend_signal_globals_id stays 0.
Registering the ini entries of the signal globals then resolves their
address through ts_resource(0), which in ZTS returns the tsrm_tls_entry
of the calling thread itself, so the write lands on the thread id stored
there. TSRM stops recognizing the thread from then on: every lookup
appends a new set of globals and tail-recurses, copying the constant,
function and class tables each time. The server never finishes booting
and its memory grows by hundreds of megabytes per second until it is
killed, with no error and no log line.

docs/compile.md and the Dockerfiles pass --disable-zend-signals, but PHP
enables Zend signals by default, so building PHP by hand without that
flag is enough to hit this. Check the flag PHP already reports through
frankenphp_get_config() and fail with an actionable error instead.

Non-ZTS builds are unaffected: their ini entries address the globals
directly, TSRM is not involved.
Comment thread frankenphp.go
return ErrInvalidPHPVersion
}

// FrankenPHP never calls zend_signal_startup(), so in ZTS the ini entries

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if calling it conditionally
would fix the issue.

@dunglas
dunglas merged commit 50781ca into php:main Sep 7, 2026
12 of 13 checks passed
@dunglas

dunglas commented Sep 7, 2026

Copy link
Copy Markdown
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants