Fix GH-23899: Handle bailout from ZipArchive cancel callback return v…

[LamentXU123]

…alidation

Now, PHP 8.6 moved archive closing from free_obj to dtor_obj as a result that callbacks can run before the executor shuts down.If a cancel callback returns an invalid type during shutdown, it can cause assertion error.

Well PHP 8.5 closes the archive in free_obj, which is already marked as called so it does not have this double-release path. Here, we just catch the bailout from return-type validation using the existing bailout_callback mechanism

[devnexen]

I think you can push this up in the zend_catch part wdyt ?

[LamentXU123]

Do you mean moving zval_ptr_dtor(&cb_retval) into zend_catch? Mind that we also need to release the return value on the normal path including when validation throws a TypeError without bailing out.

[devnexen]

yes indeed that s the way to do it, bad phrasing from me

[devnexen]

just to be sure can this test being added ?

--TEST--
GH-23899 (Cancel callback return value whose destructor throws during shutdown)
--EXTENSIONS--
zip
--SKIPIF--
<?php
if (!method_exists(ZipArchive::class, 'registerCancelCallback')) {
    die('skip cancel callbacks are not supported');
}
?>
--FILE--
<?php
class ThrowingDestructor {
    public function __destruct() {
        throw new Exception('destructor');
    }
}

$zip = new ZipArchive;
$zip->open(__DIR__ . '/gh23899_destructor.zip', ZipArchive::CREATE | ZipArchive::OVERWRITE);
$zip->registerCancelCallback(function () {
    return new ThrowingDestructor;
});
$zip->addFromString('test', 'test');
echo "Done", PHP_EOL;
?>
--CLEAN--
<?php
@unlink(__DIR__ . '/gh23899_destructor.zip');
?>
--EXPECTF--
Done

[devnexen]

seeing the nested approach, I wonder if putting the destructor inside zend_try instead would be better wdyt ?

[LamentXU123]

Yeah no strong opinions but ok :)

[devnexen]

feels right now