Ooop! We crossed over. I'll reenable 8.3.

I made a dockerfile for local testing:

https://github.com/jcupitt/docker-builds/tree/master/php-vips-php8.3

I tried some different values for the php max stacksize, but it fails with even 100gb, hilariously:

PHP Warning: Uncaught Error: Maximum call stack size of 99999950848 bytes (zend.max_allowed_stack_size - zend.reserved_stack_size) reached. Infinite recursion? in /data/php-vips/src/GObject.php:202

But using -1 works fine and (obviously) does not use that much stack.

Perhaps php 8.3.0 is not detecting stack overflow correctly for closures?

@uuf6429 can I ping you on this? Any ideas?

@dmitryuk I think this mystery is the only thing blocking a new release. If you have any ideas ...

Quick summary for anyone who hasn't been following this:

composer test fails for php 8.3 with this stack trace:

> phpunit
PHPUnit 9.6.16 by Sebastian Bergmann and contributors.
...........................................................PHP Warning: Uncaught Error: Maximum call stack size of 99999950848 bytes (zend.max_allowed_stack_size - zend.reserved_stack_size) reached. Infinite recursion? in /data/php-vips/src/GObject.php:202
Stack trace:
#0 /data/php-vips/src/VipsOperation.php(308): Jcupitt\Vips\GObject::Jcupitt\Vips\{closure}()
#1 /data/php-vips/src/VipsOperation.php(308): FFI->vips_cache_operation_build()
#2 /data/php-vips/src/VipsOperation.php(371): Jcupitt\Vips\VipsOperation::callBase()
#3 /data/php-vips/src/Image.php(1133): Jcupitt\Vips\VipsOperation::call()
#4 /data/php-vips/tests/StreamingTest.php(46): Jcupitt\Vips\Image->writeToTarget()
#5 /data/php-vips/vendor/phpunit/phpunit/src/Framework/TestCase.php(1612): Jcupitt\Vips\Test\StreamingTest->testFromSourceToTarget()
#6 /data/php-vips/vendor/phpunit/phpunit/src/Framework/TestCase.php(1218): PHPUnit\Framework\TestCase->runTest()
#7 /data/php-vips/vendor/phpunit/phpunit/src/Framework/TestResult.php(728): PHPUnit\Framework\TestCase->runBare()
#8 /data/php-vips/vendor/phpunit/phpunit/src/Framework/TestCase.php(968): PHPUnit\Framework\TestResult->run()
#9 /data/php-vips/vendor/phpunit/phpunit/src/Framework/TestSuite.php(684): PHPUnit\Framework\TestCase->run()
#10 /data/php-vips/vendor/phpunit/phpunit/src/Framework/TestSuite.php(684): PHPUnit\Framework\TestSuite->run()
#11 /data/php-vips/vendor/phpunit/phpunit/src/Framework/TestSuite.php(684): PHPUnit\Framework\TestSuite->run()
#12 /data/php-vips/vendor/phpunit/phpunit/src/Framework/TestSuite.php(684): PHPUnit\Framework\TestSuite->run()
#13 /data/php-vips/vendor/phpunit/phpunit/src/TextUI/TestRunner.php(651): PHPUnit\Framework\TestSuite->run()
#14 /data/php-vips/vendor/phpunit/phpunit/src/TextUI/Command.php(144): PHPUnit\TextUI\TestRunner->run()
#15 /data/php-vips/vendor/phpunit/phpunit/src/TextUI/Command.php(97): PHPUnit\TextUI\Command->run()
#16 /data/php-vips/vendor/phpunit/phpunit/phpunit(107): PHPUnit\TextUI\Command::main()
#17 /data/php-vips/vendor/bin/phpunit(120): include('...')
#18 {main}
 thrown in /data/php-vips/src/GObject.php on line 202
PHP Fatal error: Throwing from FFI callbacks is not allowed in /data/php-vips/src/VipsOperation.php on line 308
Script phpunit handling the test event returned with error code 255

It looks like a stack overflow is being detected in the write marshaller:

https://github.com/libvips/php-vips/blob/master/src/GObject.php#L203

It's when that closure is invoked (before any code runs). Kleis discovered that adding:

zend.max_allowed_stack_size=-1

(ie. no max stack size) to php.ini fixes it. There doesn't seem to be a positive value for max_allowed_stack_size that works.

PR php/php-src#12823 might be relevant here. Specifically comment php/php-src#12823 (comment) which mentions that the stack tracking code won't work for FFI callbacks that are called from non-main threads.

I agree, that sounds like the issue --- callbacks happening off the main thread. Good find!

I don't think we can work around this one easily.

I suppose we'll need to add zend.max_allowed_stack_size=-1 to our list of required php.ini settings. Maybe something like:

 if (PHP_MAJOR_VERSION >= 8 && PHP_MINOR_VERSION >= 3) {
 $stacksize = ini_get('zend.max_allowed_stack_size');
 if ($stacksize != '-1') {
 throw new Exception("zend.max_allowed_stack_size not set to '-1'");
 }
 }

in startup?

.... I added a version of that check, and updated the README.

@uuf6429 can I ping you on this? Any ideas?

I'm sorry, I was also clueless at that point.

The problem seemed to happen only for PHP resources being tested.

I started looking at PHP's FFI implementation and then kinda gave up after finding and fixing the (easy) assertion stuff. 😅

It might help to get someone from room 11 to look into it.

I don't think we can work around this one easily.

I think the check you propose is a reasonable one.
My only concern is that releasing it in composer means these packages will not work with future PHP 8.3+ versions (that might have been fixed). On the other hand, using a fixed PHP version or range (e.g. 8.3.0...8.3.x) will cause the opposite problem; packages will not work with future PHP releases without the fix.

The only sane solution is that you keep the check as is, but support disabling it with a flag, e.g.:

class FFI {
 /**
 * <some useful description>
 */
 public static $checkMaxStackSize = true;
 ...
 if (self::$checkMaxStackSize && version_compare(PHP_VERSION, '8.3', '>=') &&
 ini_get('zend.max_allowed_stack_size') != '-1') {
 throw new Exception("zend.max_allowed_stack_size not set to '-1'");
 }

Alternatively, but it might be too much, you could postpone that check to where it is really needed - so it doesn't fully block everyone.

PS: It might be useful to mention this GH issue number in the exception message.

My only concern is that releasing it in composer means these packages will not work with future PHP 8.3+ versions (that might have been fixed). On the other hand, using a fixed PHP version or range (e.g. 8.3.0...8.3.x) will cause the opposite problem; packages will not work with future PHP releases without the fix.

If PHP fix this in 8.4 or 8.5, we can do a new php-vips with a range check, so I think it's OK.

This code will still work even if php fix the bug, it'll just mean people have to add a line to their php.ini.

I asked for second opinions in room11 btw

@jcupitt I like to ask if we could add the check. I'm happily using your library in one of my projects with PHP 8.3. Unfortunately I can't change the ini value and it's not possible to temporary change it on runtime.
When I remove the condition for zend.max_allowed_stack_size in your source code, your library is still working for me. So I assume the PHP FFI bug you mentioned is either not occurring on every platform and environment - or it was already fixed by PHP in a minor version.

@ckilb AFAIK, this issue occurs only in GObject::signalConnect(), which is required by SourceCustom and/or TargetCustom. If you're not using signalConnect or SourceCustom/TargetCustom it's probably safe to remove this check.

That said, I can confirm that this issue persists in PHP 8.3 and the upcoming 8.4 release, see:
https://github.com/kleisauke/php-vips/actions/runs/11121526904

@kleisauke You're right, I neither use signalConnect, SourceCustom nor TargetCustom.

PR #261 moves this zend.max_allowed_stack_size=-1 check to GObject::signalConnect() instead.