Skip to content

Commit b829ea5

Browse files
committed
Fix leak when generator closed during yield in finally
In this case we need to free any pending exceptions or return values that will be discarded.
1 parent 6ec4056 commit b829ea5

File tree

2 files changed

+95
-23
lines changed

2 files changed

+95
-23
lines changed
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
--TEST--
2+
Free pending exceptions / return values on clone on yield in finally
3+
--FILE--
4+
<?php
5+
function gen1() {
6+
try {
7+
throw new Exception();
8+
} finally {
9+
yield;
10+
}
11+
}
12+
function gen2() {
13+
try {
14+
$bar = "bar";
15+
return "foo" . $bar;
16+
} finally {
17+
yield;
18+
}
19+
}
20+
function gen3() {
21+
try {
22+
throw new Exception();
23+
} finally {
24+
try {
25+
$bar = "bar";
26+
return "foo" . $bar;
27+
} finally {
28+
yield;
29+
}
30+
}
31+
}
32+
function gen4() {
33+
try {
34+
try {
35+
$bar = "bar";
36+
return "foo" . $bar;
37+
} finally {
38+
yield;
39+
}
40+
} finally {
41+
}
42+
}
43+
gen1()->rewind();
44+
gen2()->rewind();
45+
gen3()->rewind();
46+
gen4()->rewind();
47+
48+
?>
49+
===DONE===
50+
--EXPECT--
51+
===DONE===

Zend/zend_generators.c

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,10 @@ ZEND_API zend_execute_data* zend_generator_freeze_call_stack(zend_execute_data *
9595
static void zend_generator_cleanup_unfinished_execution(
9696
zend_generator *generator, zend_execute_data *execute_data, uint32_t catch_op_num) /* {{{ */
9797
{
98-
if (execute_data->opline != execute_data->func->op_array.opcodes) {
98+
zend_op_array *op_array = &execute_data->func->op_array;
99+
if (execute_data->opline != op_array->opcodes) {
99100
/* -1 required because we want the last run opcode, not the next to-be-run one. */
100-
uint32_t op_num = execute_data->opline - execute_data->func->op_array.opcodes - 1;
101+
uint32_t op_num = execute_data->opline - op_array->opcodes - 1;
101102

102103
if (UNEXPECTED(generator->frozen_call_stack)) {
103104
/* Temporarily restore generator->execute_data if it has been NULLed out already. */
@@ -106,6 +107,7 @@ static void zend_generator_cleanup_unfinished_execution(
106107
zend_generator_restore_call_stack(generator);
107108
generator->execute_data = save_ex;
108109
}
110+
109111
zend_cleanup_unfinished_execution(execute_data, op_num, catch_op_num);
110112
}
111113
}
@@ -166,7 +168,7 @@ static void zend_generator_dtor_storage(zend_object *object) /* {{{ */
166168
{
167169
zend_generator *generator = (zend_generator*) object;
168170
zend_execute_data *ex = generator->execute_data;
169-
uint32_t op_num, finally_op_num, finally_op_end;
171+
uint32_t op_num, try_catch_offset;
170172
int i;
171173

172174
/* leave yield from mode to properly allow finally execution */
@@ -194,38 +196,57 @@ static void zend_generator_dtor_storage(zend_object *object) /* {{{ */
194196
/* -1 required because we want the last run opcode, not the
195197
* next to-be-run one. */
196198
op_num = ex->opline - ex->func->op_array.opcodes - 1;
199+
try_catch_offset = -1;
197200

198-
/* Find next finally block */
199-
finally_op_num = 0;
200-
finally_op_end = 0;
201+
/* Find the innermost try/catch that we are inside of. */
201202
for (i = 0; i < ex->func->op_array.last_try_catch; i++) {
202203
zend_try_catch_element *try_catch = &ex->func->op_array.try_catch_array[i];
203-
204204
if (op_num < try_catch->try_op) {
205205
break;
206206
}
207-
208-
if (op_num < try_catch->finally_op) {
209-
finally_op_num = try_catch->finally_op;
210-
finally_op_end = try_catch->finally_end;
207+
if (op_num < try_catch->catch_op || op_num < try_catch->finally_end) {
208+
try_catch_offset = i;
211209
}
212210
}
213211

214-
/* If a finally block was found we jump directly to it and
215-
* resume the generator. */
216-
if (finally_op_num) {
217-
zval *fast_call;
212+
/* Walk try/catch/finally structures upwards, performing the necessary actions. */
213+
while (try_catch_offset != (uint32_t) -1) {
214+
zend_try_catch_element *try_catch = &ex->func->op_array.try_catch_array[try_catch_offset];
218215

219-
zend_generator_cleanup_unfinished_execution(generator, ex, finally_op_num);
216+
if (op_num < try_catch->finally_op) {
217+
/* Go to finally block */
218+
zval *fast_call =
219+
ZEND_CALL_VAR(ex, ex->func->op_array.opcodes[try_catch->finally_end].op1.var);
220220

221-
fast_call = ZEND_CALL_VAR(ex, ex->func->op_array.opcodes[finally_op_end].op1.var);
222-
Z_OBJ_P(fast_call) = EG(exception);
223-
EG(exception) = NULL;
224-
Z_OPLINE_NUM_P(fast_call) = (uint32_t)-1;
221+
zend_generator_cleanup_unfinished_execution(generator, ex, try_catch->finally_op);
222+
Z_OBJ_P(fast_call) = EG(exception);
223+
EG(exception) = NULL;
224+
Z_OPLINE_NUM_P(fast_call) = (uint32_t)-1;
225225

226-
ex->opline = &ex->func->op_array.opcodes[finally_op_num];
227-
generator->flags |= ZEND_GENERATOR_FORCED_CLOSE;
228-
zend_generator_resume(generator);
226+
ex->opline = &ex->func->op_array.opcodes[try_catch->finally_op];
227+
generator->flags |= ZEND_GENERATOR_FORCED_CLOSE;
228+
zend_generator_resume(generator);
229+
230+
/* TODO: If we hit another yield inside try/finally,
231+
* should we also jump to the next finally block? */
232+
return;
233+
} else if (op_num < try_catch->finally_end) {
234+
zval *fast_call =
235+
ZEND_CALL_VAR(ex, ex->func->op_array.opcodes[try_catch->finally_end].op1.var);
236+
/* Clean up incomplete return statement */
237+
if (Z_OPLINE_NUM_P(fast_call) != (uint32_t) -1) {
238+
zend_op *retval_op = &ex->func->op_array.opcodes[Z_OPLINE_NUM_P(fast_call)];
239+
if (retval_op->op2_type & (IS_TMP_VAR | IS_VAR)) {
240+
zval_ptr_dtor(ZEND_CALL_VAR(ex, retval_op->op2.var));
241+
}
242+
}
243+
/* Clean up backed-up exception */
244+
if (Z_OBJ_P(fast_call)) {
245+
OBJ_RELEASE(Z_OBJ_P(fast_call));
246+
}
247+
}
248+
249+
try_catch_offset--;
229250
}
230251
}
231252
/* }}} */

0 commit comments

Comments
 (0)