Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iterator_count runs indefinitely if iterator doesn't complete #8235

Closed
goodevilgenius opened this issue Mar 21, 2022 · 2 comments
Closed

iterator_count runs indefinitely if iterator doesn't complete #8235

goodevilgenius opened this issue Mar 21, 2022 · 2 comments

Comments

@goodevilgenius
Copy link

Description

The following code:

<?php
function fibonacci(): Generator
{
    yield $a = 1;
    yield $b = 2;

    start:
    yield $c = $a + $b;
    $a = $b;
    $b = $c;
    goto start;
}

$gen = fibonacci();
iterator_count($gen);

Resulted in this output:

// None. Program stalls

But I expected this output instead:

// some sort of exception

When an iterator never finishes, the iterator_count method simply hangs. This is in contrast to the iterator_to_array method, which throws a fatal error when it runs out of memory. Since iterator_count will continue running, and eventually overflow, no exception is thrown.

This is obviously a tricky situation. But it would be useful if there were some way to detect that an iterator is unlikely to finish, and throw an exception of some sort, rather than simply allowing the program to hang.

I've observed this behavior in multiple versions of PHP (8.0+), as well as multiple operating systems (desktop Linux, Android, and macOS)

PHP Version

8.1.3

Operating System

Darwin 21.3.0

@damianwadley
Copy link
Member

Normally this sort of "PHP code runs for a long time without running out of memory due to some infinite loop" problem is covered by the max_execution_time setting. Does that make sense as a solution here too?

@cmb69
Copy link
Member

cmb69 commented Mar 21, 2022

Since iterator_count will continue running, and eventually overflow, no exception is thrown.

I think we need to bail out in case of overflow:

(*(zend_long*)puser)++;

While that may not happen in practice, wrapping around certainly makes no sense.

cmb69 added a commit to cmb69/php-src that referenced this issue Apr 27, 2022
We need to prevent integer overflow to eventually stop the iteration.

A test case doesn't appear sensible for this, because even on 32bit
architectures a respective test easily runs for a few minutes.
@cmb69 cmb69 linked a pull request Apr 27, 2022 that will close this issue
@cmb69 cmb69 self-assigned this Apr 27, 2022
@cmb69 cmb69 closed this as completed in ad7b9f4 May 3, 2022
cmb69 added a commit that referenced this issue May 3, 2022
* PHP-8.0:
  Fix GH-8235: iterator_count() may run indefinitely
cmb69 added a commit that referenced this issue May 3, 2022
* PHP-8.1:
  Fix GH-8235: iterator_count() may run indefinitely
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@goodevilgenius @cmb69 @damianwadley and others