fix(types): better zval handling to avoid leaks with arrays#1780
Merged
withinboredom merged 1 commit intophp:mainfrom Aug 4, 2025
Merged
fix(types): better zval handling to avoid leaks with arrays#1780withinboredom merged 1 commit intophp:mainfrom
withinboredom merged 1 commit intophp:mainfrom
Conversation
b9ed6af to
31e5161
Compare
withinboredom
approved these changes
Aug 1, 2025
types.go
Outdated
| zval := (*C.zval)(C.__emalloc__(C.size_t(unsafe.Sizeof(C.zval{})))) | ||
| u1 := (*C.uint8_t)(unsafe.Pointer(&zval.u1[0])) | ||
| v0 := unsafe.Pointer(&zval.value[0]) | ||
| zval := C.__zval_alloc__() |
Contributor
There was a problem hiding this comment.
Suggested change
| zval := C.__zval_alloc__() | |
| var zval C.zval |
You don't need to allocate here. In order to avoid leaking, you just need to declare the zval and then pass a reference, PHP will handle allocation
var zval C.zval
switch v := value.(type) {
case nil:
C.__zval_null__(&zval)
case bool:
C.__zval_bool__(&zval, C._Bool(v))
case int:
C.__zval_long__(&zval, C.zend_long(v))
case ...When running PHP in debug mode (built with --enable-debug like in the dev.Dockerfile) you should see something like this when there is a leak between requests:
Last leak repeated 2 times
=== Total 3 memory leaks detected ===
[Fri Aug 1 21:58:55 2025] Script: '-'
Member
Author
There was a problem hiding this comment.
It's now updated. Thanks!
you should see something like this when there is a leak between requests
Should you see this in the Caddy console when running in worker mode for example?
Member
There was a problem hiding this comment.
You'll only see it when php shuts down. So you won't see it while the worker is running.
31e5161 to
647d689
Compare
henderkes
pushed a commit
to static-php/frankenphp
that referenced
this pull request
Sep 11, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related to #1771
cc @AlliBalliBaba