bukka
Repos
51
Followers
163
Following
7

Events

issue comment
Match FPM status pool's expose_php with parent

There are also some conflicts looks like so you might need to rebase this. I recommend to first squash it to a single commit and then rebase.

Created at 1 day ago
issue comment
Wrong value from ini_get() for shared files because of opcache optimization

Sorry for the late reply. I finally took a look into this and agree that it's a bit more work than I though.

I haven't done any changes in Zend ini so I might be wrong here but from looking to the code, it looks to me that php_ini_activate_per_host_config() and php_ini_activate_per_dir_config() are meant for loading the configuration during sapi activation. It means they use activate stage and allow modifying system. Think that part zend_alter_ini_entry_ex is just allow modification of any entry basically so I wouldn't think there's any change required. But would have to try it to be sure. By the way I don't think there are any docs for stages. At least I'm not aware any.

Otherwise your analysis seems correct to me. You will probably just see it if you start implementing it and things don't work... :)

I don't think disabling optimization is an option as Dmitry was against it. Also it's true that modifying some system ini should not be done after the init and might not even work for some options as it was mentioned. I think this is a better solution in general IMHO.

Created at 1 day ago
pull request closed
Make socket path shorter for socket_cmsg_{rights|credentials}.phpt

When running in CI it fails when path/address is longer 108 with following message

Fatal error: Uncaught ValueError: socket_bind(): Argument #2 ($address) must be less than 108 in ...

Created at 1 day ago
issue comment
Make socket path shorter for socket_cmsg_{rights|credentials}.phpt

Yeah I understand when this happens. I was more asking in which pipeline this is failing as it is fine in ours... Anyway I guess it might be failing in yours pipelines so I just merged to 8.0 ( https://github.com/php/php-src/commit/c58241a0030bbb2f620d0db05595f1e209b9ae82 ) as it's not a big deal to get it there anyway.

Created at 1 day ago

Add support for binary and octal number prefixes for INI settings

Closes GH-9560

Fix UPGRADING by adding DBA constants

Move object/class redundancy check into union type handling

As such a redundancy can only happen for union types

Fix GH-9556 "iterable" alias "array|Traversable" breaks PHP 8.1 code

Closes GH-9558

Make socket path shorter for ext/sockets/tests/socket_cmsg_{rights|credentials}.phpt

When running in CI it fails when path/address is longer 108

Merge branch 'PHP-8.0' into PHP-8.1

Merge branch 'PHP-8.1' into PHP-8.2

Merge branch 'PHP-8.2'

Created at 1 day ago

Make socket path shorter for ext/sockets/tests/socket_cmsg_{rights|credentials}.phpt

When running in CI it fails when path/address is longer 108

Merge branch 'PHP-8.0' into PHP-8.1

Merge branch 'PHP-8.1' into PHP-8.2

Created at 1 day ago

Make socket path shorter for ext/sockets/tests/socket_cmsg_{rights|credentials}.phpt

When running in CI it fails when path/address is longer 108

Merge branch 'PHP-8.0' into PHP-8.1

Created at 1 day ago

Make socket path shorter for ext/sockets/tests/socket_cmsg_{rights|credentials}.phpt

When running in CI it fails when path/address is longer 108

Created at 1 day ago
create branch
bukka create branch skilld-labs-socket-path-is-too-long-in-ci
Created at 1 day ago
issue comment
Make socket path shorter for socket_cmsg_{rights|credentials}.phpt

Just to clarify, where is this currently failing? Just considering what branch to merge this to. I guess if this is failing for someone already and they need to fix it, we could potentially target 8.0 but if it's just more theoretical, I might just merge it to master...

Created at 1 day ago
issue comment
expose per process metrics in fpm status

I think we might pre-format those labels and then have a single string to add there multiple times.

Created at 1 day ago
issue comment
FPM status with OpenMetrics format and FULL parameter

Nicely written down. Thanks!

It all looks reasonable to me. The last request might make some sense as you can identify when some request is taking too long but agree that logs might be better here. Specifically we have got slowlog for this purpose in FPM. I guess we can just add it as we provide that info in other formats.

Created at 1 day ago
issue comment
Fix possible leaks in SAPI.c sapi_read_post_data

I looked properly again into this to be sure we didn't leave some memory leak around. After checking again the code and doing a bit of debugging as well, I noticed that there is actually no memory leak in reality.

As mentioned SG(request_info).content_type_dup is always uninitialized at this stage (request startup). It means it's either fresh (first request) or freed in the previous request sapi_deactivate_module. So no need to do any checking of existing value. In fact it would actually do double free as it stands because sapi_deactivate_module doesn't set NULL to the freed value so there is probably still previously freed address...

In terms of content_length, it's only leaking in the part that is not actually reachable by the look of it. It's because the default_post_reader is set by php_startup_sapi_content_types which is called during the startup. So unless I missed anything, it should always be set.

For better clarity I added efree for the content_type and also marked that block as UNEXPECTED in the linked commit which also adds few more comments for further clarification.

Created at 1 day ago

Clarify memory usage and slightly improve sapi_read_post_data

This is a result of checking GH-8800 which assumed potential memory leaks here. Even though it was not the case in reality, the function deserves a bit of clarification to prevent similar attempts in the future.

Created at 1 day ago
issue comment
Make socket path shorter for socket_cmsg_{rights|credentials}.phpt

@andypost Are you actually working / running test on the platform where sys_get_temp_dir() is longer than 108? What I try to say is that on such platform all our tests won't already run (specifically the FPM ones). The sys_get_temp_dir() has been there for years and no one complained about it so I assume there are not many users that would have any issue with it. So unless there's a better solution, I would just use it here as well.

Created at 2 days ago