Fixed bug #70904 (yield from incorrectly marks valid generator as finished)

4 files changed, 61 insertions, 18 deletions

diff --git a/NEWS b/NEWS
index 94283000dd..ea2e23934c 100644
--- a/NEWS
+++ b/NEWS
@@ -19,6 +19,8 @@ PHP                                                                        NEWS
     php_register_internal_extensions). (Lior Kaplan)
   . Fixed \int (or generally every scalar type name with leading backslash)
     to not be accepted as type name. (Bob)
+  . Fixed bug #70904 (yield from incorrectly marks valid generator as finished).
+    (Bob)
 
 - Mysqlnd:
   . Fixed bug #68077 (LOAD DATA LOCAL INFILE / open_basedir restriction).
diff --git a/Zend/tests/generators/bug70904.phpt b/Zend/tests/generators/bug70904.phpt
new file mode 100644
index 0000000000..cd00e0bb34
--- /dev/null
+++ b/Zend/tests/generators/bug70904.phpt
@@ -0,0 +1,43 @@
+--TEST--
+Bug #70904 (yield from incorrectly marks valid generator as finished)
+--FILE--
+<?php
+
+function g1() {
+	yield 1;
+}
+
+function g2($g1) {
+	yield from $g1;
+	echo "reached!\n";
+	yield 2;
+}
+
+$g1 = g1();
+$g2 = g2($g1);
+
+var_dump($g2->valid());
+var_dump($g2->current());
+$g1->next();
+var_dump($g1->valid());
+var_dump($g2->valid());
+var_dump($g2->current());
+$g2->next();
+var_dump($g2->valid());
+var_dump($g2->current());
+$g2->next();
+var_dump($g2->valid());
+var_dump($g2->current());
+
+?>
+--EXPECT--
+bool(true)
+int(1)
+bool(false)
+bool(true)
+int(1)
+reached!
+bool(true)
+int(2)
+bool(false)
+NULL
diff --git a/Zend/tests/generators/multiple_yield_from_on_same_generator.phpt b/Zend/tests/generators/multiple_yield_from_on_same_generator.phpt
index 198377f8a1..f56a325684 100644
--- a/Zend/tests/generators/multiple_yield_from_on_same_generator.phpt
+++ b/Zend/tests/generators/multiple_yield_from_on_same_generator.phpt
@@ -37,5 +37,8 @@ int(4)
 int(6)
 int(3)
 int(5)
+int(3)
+int(5)
+NULL
 bool(false)
 
diff --git a/Zend/zend_generators.c b/Zend/zend_generators.c
index 3ffa82e3dd..5502633570 100644
--- a/Zend/zend_generators.c
+++ b/Zend/zend_generators.c
@@ -65,12 +65,6 @@ static void zend_generator_cleanup_unfinished_execution(zend_generator *generato
 
 ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished_execution) /* {{{ */
 {
-	zval_ptr_dtor(&generator->value);
-	ZVAL_UNDEF(&generator->value);
-
-	zval_ptr_dtor(&generator->key);
-	ZVAL_UNDEF(&generator->key);
-
 	if (UNEXPECTED(Z_TYPE(generator->values) != IS_UNDEF)) {
 		zval_ptr_dtor(&generator->values);
 		ZVAL_UNDEF(&generator->values);
@@ -170,6 +164,10 @@ static void zend_generator_free_storage(zend_object *object) /* {{{ */
 
 	zend_generator_close(generator, 0);
 
+	/* we can't immediately free them in zend_generator_close() else yield from won't be able to fetch it */
+	zval_ptr_dtor(&generator->value);
+	zval_ptr_dtor(&generator->key);
+
 	if (EXPECTED(!Z_ISUNDEF(generator->retval))) {
 		zval_ptr_dtor(&generator->retval);
 	}
@@ -508,6 +506,7 @@ ZEND_API zend_generator *zend_generator_update_current(zend_generator *generator
 
 						EG(current_execute_data) = original_execute_data;
 					} else {
+						ZVAL_COPY(&root->value, &root->node.parent->value);
 						ZVAL_COPY(ZEND_CALL_VAR(root->execute_data, yield_from->result.var), &root->node.parent->retval);
 					}
 				}
@@ -630,7 +629,7 @@ try_again:
 		 * after the "yield from" expression. */
 	}
 
-	if (UNEXPECTED((orig_generator->flags & ZEND_GENERATOR_DO_INIT) != 0) && !Z_ISUNDEF(generator->value)) {
+	if (UNEXPECTED((orig_generator->flags & ZEND_GENERATOR_DO_INIT) != 0 && !Z_ISUNDEF(generator->value))) {
 		/* We must not advance Generator if we yield from a Generator being currently run */
 		return;
 	}
@@ -708,7 +707,7 @@ try_again:
 
 static void inline zend_generator_ensure_initialized(zend_generator *generator) /* {{{ */
 {
-	if (EXPECTED(generator->execute_data) && UNEXPECTED(Z_TYPE(generator->value) == IS_UNDEF) && EXPECTED(generator->node.parent == NULL)) {
+	if (UNEXPECTED(Z_TYPE(generator->value) == IS_UNDEF) && EXPECTED(generator->execute_data) && EXPECTED(generator->node.parent == NULL)) {
 		generator->flags |= ZEND_GENERATOR_DO_INIT;
 		zend_generator_resume(generator);
 		generator->flags &= ~ZEND_GENERATOR_DO_INIT;
@@ -759,7 +758,7 @@ ZEND_METHOD(Generator, valid)
 
 	zend_generator_get_current(generator);
 
-	RETURN_BOOL(EXPECTED(Z_TYPE(generator->value) != IS_UNDEF || generator->node.parent != NULL));
+	RETURN_BOOL(EXPECTED(generator->execute_data != NULL));
 }
 /* }}} */
 
@@ -778,7 +777,7 @@ ZEND_METHOD(Generator, current)
 	zend_generator_ensure_initialized(generator);
 
 	root = zend_generator_get_current(generator);
-	if (EXPECTED(Z_TYPE(root->value) != IS_UNDEF)) {
+	if (EXPECTED(generator->execute_data != NULL && Z_TYPE(root->value) != IS_UNDEF)) {
 		zval *value = &root->value;
 
 		ZVAL_DEREF(value);
@@ -802,7 +801,7 @@ ZEND_METHOD(Generator, key)
 	zend_generator_ensure_initialized(generator);
 
 	root = zend_generator_get_current(generator);
-	if (EXPECTED(Z_TYPE(root->key) != IS_UNDEF)) {
+	if (EXPECTED(generator->execute_data != NULL && Z_TYPE(root->key) != IS_UNDEF)) {
 		zval *key = &root->key;
 
 		ZVAL_DEREF(key);
@@ -865,7 +864,7 @@ ZEND_METHOD(Generator, send)
 	zend_generator_resume(generator);
 
 	root = zend_generator_get_current(generator);
-	if (EXPECTED(Z_TYPE(root->value) != IS_UNDEF)) {
+	if (EXPECTED(generator->execute_data)) {
 		zval *value = &root->value;
 
 		ZVAL_DEREF(value);
@@ -905,7 +904,7 @@ ZEND_METHOD(Generator, throw)
 		zend_generator_resume(generator);
 
 		root = zend_generator_get_current(generator);
-		if (Z_TYPE(root->value) != IS_UNDEF) {
+		if (generator->execute_data) {
 			zval *value = &root->value;
 
 			ZVAL_DEREF(value);
@@ -982,11 +981,7 @@ static int zend_generator_iterator_valid(zend_object_iterator *iterator) /* {{{
 
 	zend_generator_get_current(generator);
 
-	if (EXPECTED(Z_TYPE(generator->value) != IS_UNDEF || generator->node.parent != NULL)) {
-		return SUCCESS;
-	} else {
-		return FAILURE;
-	}
+	return generator->execute_data ? SUCCESS : FAILURE;
 }
 /* }}} */