Permalink
Browse files

Properly free resources when generator return value not used

To keep things clean two new functions are introduced:

zend_clean_and_cache_symbol_table(HashTable *symbol_table)
zend_free_compiled_variables(zval ***CVs, int num)
  • Loading branch information...
1 parent bcc7d97 commit 4aab08b64c2318775be6d8e629747ac66a108485 @nikic nikic committed May 28, 2012
@@ -0,0 +1,13 @@
+--TEST--
+There shouldn't be any leaks when the genertor's return value isn't used
+--FILE--
+<?php
+
+function *gen($foo) {}
+
+gen('foo'); // return value not used
+
+?>
+===DONE===
+--EXPECT--
+===DONE===
View
@@ -1538,6 +1538,31 @@ ZEND_API zval **zend_get_zval_ptr_ptr(int op_type, const znode_op *node, const t
return get_zval_ptr_ptr(op_type, node, Ts, should_free, type);
}
+void zend_clean_and_cache_symbol_table(HashTable *symbol_table) /* {{{ */
+{
+ if (EG(symtable_cache_ptr) >= EG(symtable_cache_limit)) {
+ zend_hash_destroy(symbol_table);
+ FREE_HASHTABLE(symbol_table);
+ } else {
+ /* clean before putting into the cache, since clean
+ could call dtors, which could use cached hash */
+ zend_hash_clean(symbol_table);
+ *(++EG(symtable_cache_ptr)) = symbol_table;
+ }
+}
+/* }}} */
+
+void zend_free_compiled_variables(zval ***CVs, int num) /* {{{ */
+{
+ int i;
+ for (i = 0; i < num; ++i) {
+ if (CVs[i]) {
+ zval_ptr_dtor(CVs[i]);
+ }
+ }
+}
+/* }}} */
+
/*
* Local variables:
* tab-width: 4
View
@@ -432,6 +432,9 @@ ZEND_API zval **zend_get_zval_ptr_ptr(int op_type, const znode_op *node, const t
ZEND_API int zend_do_fcall(ZEND_OPCODE_HANDLER_ARGS);
+void zend_clean_and_cache_symbol_table(HashTable *symbol_table);
+void zend_free_compiled_variables(zval ***CVs, int num);
+
#define CACHED_PTR(num) \
EG(active_op_array)->run_time_cache[(num)]
View
@@ -32,12 +32,7 @@ void zend_generator_close(zend_generator *generator, zend_bool finished_executio
zend_execute_data *execute_data = generator->execute_data;
if (!execute_data->symbol_table) {
- int i;
- for (i = 0; i < execute_data->op_array->last_var; ++i) {
- if (execute_data->CVs[i]) {
- zval_ptr_dtor(execute_data->CVs[i]);
- }
- }
+ zend_free_compiled_variables(execute_data->CVs, execute_data->op_array->last_var);
} else {
if (EG(symtable_cache_ptr) >= EG(symtable_cache_limit)) {
zend_hash_destroy(execute_data->symbol_table);
View
@@ -2514,14 +2514,7 @@ ZEND_VM_HELPER(zend_leave_helper, ANY, ANY)
EG(current_execute_data) = EX(prev_execute_data);
EG(opline_ptr) = NULL;
if (!EG(active_symbol_table)) {
- zval ***cv = EX_CVs();
- zval ***end = cv + op_array->last_var;
- while (cv != end) {
- if (*cv) {
- zval_ptr_dtor(*cv);
- }
- cv++;
- }
+ zend_free_compiled_variables(EX_CVs(), op_array->last_var);
}
if ((op_array->fn_flags & ZEND_ACC_CLOSURE) && op_array->prototype) {
@@ -2579,15 +2572,7 @@ ZEND_VM_HELPER(zend_leave_helper, ANY, ANY)
EG(active_op_array) = EX(op_array);
EG(return_value_ptr_ptr) = EX(original_return_value);
if (EG(active_symbol_table)) {
- if (EG(symtable_cache_ptr)>=EG(symtable_cache_limit)) {
- zend_hash_destroy(EG(active_symbol_table));
- FREE_HASHTABLE(EG(active_symbol_table));
- } else {
- /* clean before putting into the cache, since clean
- could call dtors, which could use cached hash */
- zend_hash_clean(EG(active_symbol_table));
- *(++EG(symtable_cache_ptr)) = EG(active_symbol_table);
- }
+ zend_clean_and_cache_symbol_table(EG(active_symbol_table));
}
EG(active_symbol_table) = EX(symbol_table);
@@ -2732,15 +2717,7 @@ ZEND_VM_HELPER(zend_do_fcall_common_helper, ANY, ANY)
EG(active_op_array) = EX(op_array);
EG(return_value_ptr_ptr) = EX(original_return_value);
if (EG(active_symbol_table)) {
- if (EG(symtable_cache_ptr)>=EG(symtable_cache_limit)) {
- zend_hash_destroy(EG(active_symbol_table));
- FREE_HASHTABLE(EG(active_symbol_table));
- } else {
- /* clean before putting into the cache, since clean
- could call dtors, which could use cached hash */
- zend_hash_clean(EG(active_symbol_table));
- *(++EG(symtable_cache_ptr)) = EG(active_symbol_table);
- }
+ zend_clean_and_cache_symbol_table(EG(active_symbol_table));
}
EG(active_symbol_table) = EX(symbol_table);
} else { /* ZEND_OVERLOADED_FUNCTION */
@@ -5242,7 +5219,12 @@ ZEND_VM_HANDLER(159, ZEND_SUSPEND_AND_RETURN_GENERATOR, ANY, ANY)
/* if there is no return value pointer we are responsible for freeing the
* execution data */
if (!EG(return_value_ptr_ptr)) {
- /* something has to be done in here, not sure yet what exactly */
+ if (!EG(active_symbol_table)) {
+ zend_free_compiled_variables(EX_CVs(), execute_data->op_array->last_var);
+ } else {
+ zend_clean_and_cache_symbol_table(EG(active_symbol_table));
+ }
+ efree(execute_data);
}
EG(opline_ptr) = NULL;
View
@@ -497,14 +497,7 @@ static int ZEND_FASTCALL zend_leave_helper_SPEC(ZEND_OPCODE_HANDLER_ARGS)
EG(current_execute_data) = EX(prev_execute_data);
EG(opline_ptr) = NULL;
if (!EG(active_symbol_table)) {
- zval ***cv = EX_CVs();
- zval ***end = cv + op_array->last_var;
- while (cv != end) {
- if (*cv) {
- zval_ptr_dtor(*cv);
- }
- cv++;
- }
+ zend_free_compiled_variables(EX_CVs(), op_array->last_var);
}
if ((op_array->fn_flags & ZEND_ACC_CLOSURE) && op_array->prototype) {
@@ -562,15 +555,7 @@ static int ZEND_FASTCALL zend_leave_helper_SPEC(ZEND_OPCODE_HANDLER_ARGS)
EG(active_op_array) = EX(op_array);
EG(return_value_ptr_ptr) = EX(original_return_value);
if (EG(active_symbol_table)) {
- if (EG(symtable_cache_ptr)>=EG(symtable_cache_limit)) {
- zend_hash_destroy(EG(active_symbol_table));
- FREE_HASHTABLE(EG(active_symbol_table));
- } else {
- /* clean before putting into the cache, since clean
- could call dtors, which could use cached hash */
- zend_hash_clean(EG(active_symbol_table));
- *(++EG(symtable_cache_ptr)) = EG(active_symbol_table);
- }
+ zend_clean_and_cache_symbol_table(EG(active_symbol_table));
}
EG(active_symbol_table) = EX(symbol_table);
@@ -715,15 +700,7 @@ static int ZEND_FASTCALL zend_do_fcall_common_helper_SPEC(ZEND_OPCODE_HANDLER_AR
EG(active_op_array) = EX(op_array);
EG(return_value_ptr_ptr) = EX(original_return_value);
if (EG(active_symbol_table)) {
- if (EG(symtable_cache_ptr)>=EG(symtable_cache_limit)) {
- zend_hash_destroy(EG(active_symbol_table));
- FREE_HASHTABLE(EG(active_symbol_table));
- } else {
- /* clean before putting into the cache, since clean
- could call dtors, which could use cached hash */
- zend_hash_clean(EG(active_symbol_table));
- *(++EG(symtable_cache_ptr)) = EG(active_symbol_table);
- }
+ zend_clean_and_cache_symbol_table(EG(active_symbol_table));
}
EG(active_symbol_table) = EX(symbol_table);
} else { /* ZEND_OVERLOADED_FUNCTION */
@@ -1224,7 +1201,12 @@ static int ZEND_FASTCALL ZEND_SUSPEND_AND_RETURN_GENERATOR_SPEC_HANDLER(ZEND_OP
/* if there is no return value pointer we are responsible for freeing the
* execution data */
if (!EG(return_value_ptr_ptr)) {
- /* something has to be done in here, not sure yet what exactly */
+ if (!EG(active_symbol_table)) {
+ zend_free_compiled_variables(EX_CVs(), execute_data->op_array->last_var);
+ } else {
+ zend_clean_and_cache_symbol_table(EG(active_symbol_table));
+ }
+ efree(execute_data);
}
EG(opline_ptr) = NULL;

0 comments on commit 4aab08b

Please sign in to comment.