Skip to content

Free each thread's strtod Bigint pool - #21

Closed
EdmondDantes wants to merge 1 commit into
true-asyncfrom
fix/strtod-thread-freelist
Closed

Free each thread's strtod Bigint pool#21
EdmondDantes wants to merge 1 commit into
true-asyncfrom
fix/strtod-thread-freelist

Conversation

@EdmondDantes

Copy link
Copy Markdown

What was broken

Bfree() does not release a Bigint; it pushes it onto freelist[k], which under ZTS is EG(strtod_state).freelist — one pool per thread. The only thing that empties a pool is destroy_freelist(), reachable solely through zend_shutdown_strtod(), which runs once, on the main thread. executor_globals_ctor zeroes the state for every new thread and executor_globals_dtor never touched it.

So every thread that formatted or parsed a double leaked its pooled Bigints when its TSRM storage went. Measured with a ThreadPool worker doing one sprintf('%6.3f', 1.5):

68 bytes in 2 blocks are definitely lost

— a 36-byte Balloc(1) from d2b() and a 32-byte Balloc(0) from rv_alloc(). Four workers, one float each: 272 bytes in 8 blocks, exactly linear. It is small per thread and unbounded per process: a pool that rotates its workers pays it again on every rotation.

The change

destroy_freelist() and free_p5s() take the state to free instead of reaching through EG(), and executor_globals_dtor() frees the departing thread's pool. The state has to travel as an argument: TSRM runs a thread's storage destructor from whichever thread performs the shutdown, so EG() there belongs to somebody else.

zend_shutdown_strtod() keeps its signature and now goes through the same path for the main thread. Running it before the destructor is safe — the teardown NULLs both lists, so the second pass finds nothing.

The two #undefs before the teardown are needed because freelist and p5s are macros over EG(strtod_state) for the rest of the file, and they would otherwise swallow the field names in state->freelist.

Measurements

before after
one pool worker formatting one double 68 bytes definitely lost in 2 blocks 0
four workers, one double each 272 in 8 0

Zend/tests: 5398 passed, 129 skipped, 0 failed. ext/standard/tests/math: 164 passed, 0 failed. Build is ZTS, debug.

Found while chasing a leak in another project against this fork; the same code is in php-src master, so this is not fork-specific.

Bfree recycles a Bigint onto a per-thread freelist instead of releasing
it, and zend_shutdown_strtod() only ever reaches the main thread's pool.
Under ZTS every other thread that formatted or parsed a double left its
Bigints behind when its storage went: measured as 68 bytes definitely
lost per worker thread for a single sprintf('%6.3f'), scaling linearly
with the number of threads.

The freelist teardown now takes the state to free, because TSRM runs a
thread's storage destructor from whichever thread performs the shutdown,
and EG() there belongs to somebody else.
@EdmondDantes

Copy link
Copy Markdown
Author

Closing: this is upstream php-src behaviour, not a fork concern, and this repository is not where we carry such patches.

@EdmondDantes
EdmondDantes deleted the fix/strtod-thread-freelist branch August 14, 2026 18:50
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.

1 participant