Skip to content

#254: stop the worker from pinning every task's nested closure - #257

Merged
EdmondDantes merged 3 commits into
mainfrom
254-worker-retains-nested-closures
Aug 20, 2026
Merged

#254: stop the worker from pinning every task's nested closure#257
EdmondDantes merged 3 commits into
mainfrom
254-worker-retains-nested-closures

Conversation

@EdmondDantes

Copy link
Copy Markdown
Contributor

Closes #254.

What was wrong

ZEND_DECLARE_LAMBDA_FUNCTION memoizes the Closure it creates: it takes a reference, writes the object into a run-time cache slot and pushes it onto EG(lambda_cache). That stack is drained in one place, shutdown_executor, when the request ends.

A pool worker runs one request for its whole life. Every task is materialized into an op_array of its own with a cache of its own, so the slot is empty at the first execution and a new Closure is pinned — once per task, for as long as the worker accepts tasks. The pinned object holds dynamic_func_defs[i]->refcount at 1, destroy_op_array returns early, and that task's opcodes, literals, vars, arg_info, attributes and static variables are never freed.

Measured in the worker, task body $f = static function () { return 1; }; return $f();:

after  1000 tasks: 1421520 bytes
after  2000 tasks: 2349904
after  3000 tasks: 3286480
after  6000 tasks: 6079824

About 930 bytes per task, linear, no ceiling. Present on main before #251 with the same numbers, so it is older than that work.

What changed

The memo is turned off in a materialized op_array: extended_value on every ZEND_DECLARE_LAMBDA_FUNCTION is set to (uint32_t) -1, the value the compiler leaves when caching does not apply.

That is the decision zend_compile_func_decl already makes for the top level of a script, and its comment gives the same reason: "Don't cache closures in main, as those would leak without a proper cleanup mechanism." A worker's per-task op_array is the same situation. The memo buys nothing there in any case — the cache dies with the task that owns it, so nothing is ever reused.

Releasing the pinned closures at task end was the other candidate and does not work: the handler reads the cache slot before it creates anything, so a closure that outlived its task would hand out a freed object on the next call.

The price

Inside a task, the same closure literal evaluated twice yields two objects; in the submitting thread it yields one. Asserted in the test rather than left to be discovered.

Verification

  • tests/thread_pool/102-worker_releases_nested_closures.phpt — new. Red 3 of 3 on revert, green after. It warms up 200 tasks, reads memory_get_usage() in the worker, runs 2000 more and asserts the growth is under 100 KB; before the change that run grew by about 1.9 MB. Measured after the change: 502192 bytes on all six sample points, flat.
  • tests 1192 passed / 0 failed, with OPcache enabled as well; fuzzy-tests/_generated 724 / 0.

Build: ZTS DEBUG, TrueAsync ABI v0.24.0.

DECLARE_LAMBDA_FUNCTION memoizes the Closure it creates and pins it in
EG(lambda_cache), which is drained when the request ends. A worker's
request outlives every task it runs, so the pinned object held the task's
nested body at refcount 1 and destroy_op_array left the whole task op_array
behind. Measured at about 930 bytes per task, linear: 6 MB over 6000
submits, flat after the change.

Releasing the pinned closures at task end would not do: the handler reads
the cache slot before creating anything, so a closure that outlived its
task would hand out a freed object on the next call. The memo is turned off
instead, which is what zend_compile_func_decl already does for the top
level of a script, for the same reason and with the same trade: the same
closure literal evaluated twice yields two objects.
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

The test compared the worker's memory against a reading taken 200 tasks in,
so anything a worker allocates once on its way up counted against the
bound, and the release build on CI crossed it. Two windows of a thousand
tasks are compared instead: a one-off allocation lands in the first, a
per-task leak shows in both. The numbers are printed when the bound is
crossed, so a failure says how much rather than only that.
The literal allocation split by #255 and the lambda memo of this branch meet in the same block: the split stays, the memo loop moves below the memcpy that both need.
@EdmondDantes
EdmondDantes merged commit fcc6be5 into main Aug 20, 2026
9 checks passed
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.

A ThreadPool worker retains every task's nested closure until its request ends

1 participant