Cleanup generator closing code a bit

All code dealing with unfinished execution cleanup is now in a separate
function (previously most of it was run even when execution was properly
finished.

Furthermore some code dealing with unclean shutdowns has been removed,
which is no longer necessary, because we no longer try to clean up in
this case.

Author: nikic

Zend/zend_generators.c

@@ -29,6 +29,73 @@ static zend_object_handlers zend_generator_handlers;
 
 static zend_object_value zend_generator_create(zend_class_entry *class_type TSRMLS_DC);
 
+static void zend_generator_cleanup_unfinished_execution(zend_generator *generator TSRMLS_DC) /* {{{ */
+{
+	zend_execute_data *execute_data = generator->execute_data;
+	zend_op_array *op_array = execute_data->op_array;
+
+	if (generator->send_target) {
+		Z_DELREF_PP(generator->send_target);
+		generator->send_target = NULL;
+	}
+
+	/* Manually free loop variables, as execution couldn't reach their
+	 * SWITCH_FREE / FREE opcodes. */
+	{
+		/* -1 required because we want the last run opcode, not the
+		 * next to-be-run one. */
+		zend_uint op_num = execute_data->opline - op_array->opcodes - 1;
+
+		int i;
+		for (i = 0; i < op_array->last_brk_cont; ++i) {
+			zend_brk_cont_element *brk_cont = op_array->brk_cont_array + i;
+
+			if (brk_cont->start < 0) {
+				continue;
+			} else if (brk_cont->start > op_num) {
+				break;
+			} else if (brk_cont->brk > op_num) {
+				zend_op *brk_opline = op_array->opcodes + brk_cont->brk;
+
+				switch (brk_opline->opcode) {
+					case ZEND_SWITCH_FREE:
+						{
+							temp_variable *var = EX_TMP_VAR(execute_data, brk_opline->op1.var);
+							zval_ptr_dtor(&var->var.ptr);
+						}
+						break;
+					case ZEND_FREE:
+						{
+							temp_variable *var = EX_TMP_VAR(execute_data, brk_opline->op1.var);
+							zval_dtor(&var->tmp_var);
+						}
+						break;
+				}
+			}
+		}
+	}
+
+	/* Clear any backed up stack arguments */
+	{
+		void **ptr = generator->stack->top - 1;
+		void **end = zend_vm_stack_frame_base(execute_data);
+
+		for (; ptr >= end; --ptr) {
+			zval_ptr_dtor((zval **) ptr);
+		}
+	}
+
+	/* If yield was used as a function argument there may be active
+	 * method calls those objects need to be freed */
+	while (execute_data->call >= execute_data->call_slots) {
+		if (execute_data->call->object) {
+			zval_ptr_dtor(&execute_data->call->object);
+		}
+		execute_data->call--;
+	}
+}
+/* }}} */
+
 ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished_execution TSRMLS_DC) /* {{{ */
 {
 	if (generator->value) {
@@ -55,76 +122,12 @@ ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished
 			zval_ptr_dtor(&execute_data->current_this);
 		}
 
-		if (!finished_execution && generator->send_target) {
-			Z_DELREF_PP(generator->send_target);
-			generator->send_target = NULL;
-		}
-
 		/* A fatal error / die occurred during the generator execution. Trying to clean
 		 * up the stack may not be safe in this case. */
 		if (CG(unclean_shutdown)) {
 			return;
 		}
 
-		/* If the generator is closed before it can finish execution (reach
-		 * a return statement) we have to free loop variables manually, as
-		 * we don't know whether the SWITCH_FREE / FREE opcodes have run */
-		if (!finished_execution) {
-			/* -1 required because we want the last run opcode, not the
-			 * next to-be-run one. */
-			zend_uint op_num = execute_data->opline - op_array->opcodes - 1;
-
-			int i;
-			for (i = 0; i < op_array->last_brk_cont; ++i) {
-				zend_brk_cont_element *brk_cont = op_array->brk_cont_array + i;
-
-				if (brk_cont->start < 0) {
-					continue;
-				} else if (brk_cont->start > op_num) {
-					break;
-				} else if (brk_cont->brk > op_num) {
-					zend_op *brk_opline = op_array->opcodes + brk_cont->brk;
-
-					switch (brk_opline->opcode) {
-						case ZEND_SWITCH_FREE:
-							{
-								temp_variable *var = EX_TMP_VAR(execute_data, brk_opline->op1.var);
-								zval_ptr_dtor(&var->var.ptr);
-							}
-							break;
-						case ZEND_FREE:
-							{
-								temp_variable *var = EX_TMP_VAR(execute_data, brk_opline->op1.var);
-								zval_dtor(&var->tmp_var);
-							}
-							break;
-					}
-				}
-			}
-		}
-
-		/* Clear any backed up stack arguments */
-		if (generator->stack != EG(argument_stack)) {
-			void **ptr = generator->stack->top - 1;
-			void **end = zend_vm_stack_frame_base(execute_data);
-
-			/* If the top stack element is the argument count, skip it */
-			if (execute_data->function_state.arguments) {
-				ptr--;
-			}
-
-			for (; ptr >= end; --ptr) {
-				zval_ptr_dtor((zval**) ptr);
-			}
-		}
-
-		while (execute_data->call >= execute_data->call_slots) {
-			if (execute_data->call->object) {
-				zval_ptr_dtor(&execute_data->call->object);
-			}
-			execute_data->call--;
-		}
-
 		/* We have added an additional stack frame in prev_execute_data, so we
 		 * have to free it. It also contains the arguments passed to the
 		 * generator (for func_get_args) so those have to be freed too. */
@@ -143,17 +146,19 @@ ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished
 			}
 		}
 
+		/* Some cleanups are only necessary if the generator was closued
+		 * before it could finish execution (reach a return statement). */
+		if (!finished_execution) {
+			zend_generator_cleanup_unfinished_execution(generator TSRMLS_CC);
+		}
+
 		/* Free a clone of closure */
 		if (op_array->fn_flags & ZEND_ACC_CLOSURE) {
 			destroy_op_array(op_array TSRMLS_CC);
 			efree(op_array);
 		}
 
 		efree(generator->stack);
-		if (generator->stack == EG(argument_stack)) {
-			/* abnormal exit for running generator */
-			EG(argument_stack) = NULL;
-		}
 		generator->execute_data = NULL;
 	}
 }