[lazerg]

With CATCH_GET_CHILD, RecursiveIteratorIterator and RecursiveCachingIterator call zend_clear_exception() on any exception thrown by hasChildren(), getChildren(), beginChildren() and friends. That also swallows the unwind exit from exit() and the graceful exit used when destroying fibers, so the script keeps running. These exceptions are now passed through as if CATCH_GET_CHILD was not set.

Fixes #24264

[Girgias]

I do not think we should even be using zend_clear_exception.

[lazerg]

What would you use instead? CATCH_GET_CHILD is meant to ignore exceptions from getChildren()/hasChildren(), so the exception has to be dropped somewhere. Do you want it to catch only Exception and let Error through, or something else?

[ndossche]

The C code is supposed to do the equivalent of a PHP try-catch that throws away the exception. That matches the semantics description of CATCH_GET_CHILD.
We should only be doing a zend_clear_exception in case it's not one of the uncatchable exceptions, so in that sense the PR is correct.
Please expand the tests a bit, e.g. at least the getChildren() case.

[lazerg]

I added getChildren() tests for RecursiveIteratorIterator and RecursiveCachingIterator in cc75dea. I also added one where a fiber is destroyed while it is suspended in getChildren(), so the graceful exit path has a test too.

[ndossche]

Can you please drop the NEWS change? This is something we always do ourselves when upmerging anyway, and it is blocking CI runs on this PR due to the merge conflict

[lazerg]

Done, I removed the NEWS entry in a2e52dd.

[Girgias]

SPL is truly the gift that keeps on giving.

[Girgias]

Can you pass the flags as a parameter so the !(object->flags & RIT_CATCH_GET_CHILD) logic can be moved into this function.

[lazerg]

Done in ed3c6f0. The function now takes the flags and checks CATCH_GET_CHILD itself, so both iterators use it the same way.

[ndossche]

I think this version is slightly worse than the version with flags separate because now it depends on this implementation detail:

php-src/ext/spl/spl_iterators.h

Line 50 in 17c806d

#define RIT_CATCH_GET_CHILD CIT_CATCH_GET_CHILD

However, this will work, and I can't be bothered to fight over this. Probably other code already depends on this anyway

[ndossche]

Merged, thanks