Skip to content

Commit

Permalink
Simplify and fix generator tree management
Browse files Browse the repository at this point in the history
This makes a number of related changes to the generator tree
management, that should hopefully make it easier to understand,
more robust and faster for the common linear-chain case. Fixes
https://bugs.php.net/bug.php?id=80240, which was the original
motivation here.

 * Generators now only add a ref to their direct parent.
 * Nodes only store their children, not their leafs, which avoids
   any need for leaf updating. This means it's no longer possible
   to fetch the child for a certain leaf, which is something we
   only needed in one place (update_current). If multi-children
   nodes are involved, this will require doing a walk in the other
   direction (from leaf to root). It does not affect the common
   case of single-child nodes.
 * The root/leaf pointers are now seen as a pair. One leaf generator
   can point to the current root. If a different leaf generator is
   used, we'll move the root pointer over to that one. Again, this
   is a cache to make the common linear chain case fast, trees may
   need to scan up the parent link.

Closes GH-6344.
  • Loading branch information
nikic committed Oct 22, 2020
1 parent ac87880 commit dd4a080
Show file tree
Hide file tree
Showing 5 changed files with 221 additions and 258 deletions.
32 changes: 32 additions & 0 deletions Zend/tests/generators/backtrace_multi_yield_from.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
--TEST--
Generator backtrace with multi yield from
--FILE--
<?php

function gen() {
yield 1;
debug_print_backtrace();
yield 2;
}

function from($gen) {
yield from $gen;
}

$gen1 = gen();
$gen2 = from($gen1);
$gen3 = from($gen2);
var_dump($gen3->current());
$gen2->next();
var_dump($gen2->current());
$gen2->next();
var_dump($gen2->current());

?>
--EXPECTF--
int(1)
int(1)
#0 gen() called at [%s:10]
#1 from(Generator Object ())
#2 Generator->next() called at [%s:19]
int(2)
28 changes: 28 additions & 0 deletions Zend/tests/generators/bug80240.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
--TEST--
Bug #80240: Use after free multi yield from
--FILE--
<?php

function gen() {
yield 0;
yield from gen();
}

function bar($gen) {
yield from $gen;
}

$gen = gen();
$a = bar($gen);
$b = bar($gen);
$a->rewind();
$b->rewind();
$a->next();
unset($gen);
unset($a);
unset($b);

?>
===DONE===
--EXPECT--
===DONE===
24 changes: 24 additions & 0 deletions Zend/tests/generators/yield_from_chain_dtor_order.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
--TEST--
Leaf link may need to be invalidated depending on dtor order
--FILE--
<?php

function gen2() {
yield 1;
}
function gen() {
yield from gen2();
}
function bar($g) {
yield from $g;
}

$gen = gen();
$bar = bar($gen);
var_dump($bar->current());
$copy = $bar;
unset($gen);

?>
--EXPECT--
int(1)

1 comment on commit dd4a080

@carusogabriel
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.