Skip to content

Conversation

@alexandre-daubois
Copy link
Member

Related to #1771

cc @AlliBalliBaba

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__()
Copy link
Contributor

@AlliBalliBaba AlliBalliBaba Aug 1, 2025

Choose a reason for hiding this comment

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

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:  '-'

Copy link
Member Author

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

You'll only see it when php shuts down. So you won't see it while the worker is running.

Copy link
Contributor

@AlliBalliBaba AlliBalliBaba left a comment

Choose a reason for hiding this comment

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

👍

@withinboredom withinboredom merged commit 1d0169d into php:main Aug 4, 2025
43 checks passed
henderkes pushed a commit to static-php/frankenphp that referenced this pull request Sep 11, 2025
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.

3 participants