Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 185c7fa

Browse files
nikicnikic
committedApr 9, 2013
Initial patch for interruption safe external array iterators
Currently there are two ways to iterate an array: Either using the internal array pointer or using an external HashPosition. Right now the latter isn't interruption safe though and can't be used in any iteration that runs user code. For that reason foreach had to use the IAP for the iteration. This created a bunch of issues: Firstly foreach is often required to copy the array it iterates even though it should not be strictly necessary according to COW. Secondly using the IAP created weird behavior of current() etc in the loop body, that was furthermore heavily dependent on just how exactly the looping was done. Thirdly the behavior when modifying the array during iteration is very unpredictable. This patch approaches the problem by making external HashPosition array iterators interruption safe. This is done by adding two new APIs: void zend_track_hash_position(HashTable *ht, HashPosition *pos); void zend_untrack_hash_position(HashTable *ht, HashPosition *pos); Using this functions the HashPosition has to be registered before the iteration and unregistered after it. If the HashPosition is registered in such a way the zend_hash operations will properly update the HashPosition pointer on modification, just like it is usually done for the IAP.
1 parent 1a8575a commit 185c7fa

18 files changed

+457
-551
lines changed
 

‎Zend/tests/bug40509.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,4 @@ var_dump(key($arr["v"]));
2323
int(0)
2424
int(0)
2525
int(0)
26-
NULL
26+
int(0)

‎Zend/tests/bug40705.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,4 @@ int(0)
2323
int(0)
2424
int(1)
2525
int(2)
26-
NULL
26+
int(0)

‎Zend/zend_compile.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2678,7 +2678,7 @@ static int generate_free_foreach_copy(const zend_op *foreach_copy TSRMLS_DC) /*
26782678

26792679
opline = get_next_op(CG(active_op_array) TSRMLS_CC);
26802680

2681-
opline->opcode = (foreach_copy->result_type == IS_TMP_VAR) ? ZEND_FREE : ZEND_SWITCH_FREE;
2681+
opline->opcode = ZEND_FOREACH_FREE;
26822682
COPY_NODE(opline->op1, foreach_copy->result);
26832683
SET_UNUSED(opline->op2);
26842684
opline->extended_value = 1;

‎Zend/zend_execute.c

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1409,6 +1409,45 @@ static void zend_fetch_property_address(temp_variable *result, zval **container_
14091409
}
14101410
}
14111411

1412+
static inline void i_handle_free_op(zend_op *opline, const zend_execute_data *execute_data TSRMLS_DC) /* {{{ */
1413+
{
1414+
switch (opline->opcode) {
1415+
case ZEND_SWITCH_FREE:
1416+
if (!(opline->extended_value & EXT_TYPE_FREE_ON_RETURN)) {
1417+
zval_ptr_dtor(&EX_T(opline->op1.var).var.ptr);
1418+
}
1419+
break;
1420+
case ZEND_FREE:
1421+
if (!(opline->extended_value & EXT_TYPE_FREE_ON_RETURN)) {
1422+
zendi_zval_dtor(EX_T(opline->op1.var).tmp_var);
1423+
}
1424+
break;
1425+
case ZEND_FOREACH_FREE:
1426+
if (!(opline->extended_value & EXT_TYPE_FREE_ON_RETURN)) {
1427+
zval *array = EX_T(opline->op1.var).fe.ptr;
1428+
zend_object_iterator *iter_dummy;
1429+
1430+
switch (zend_iterator_unwrap(array, &iter_dummy TSRMLS_CC)) {
1431+
case ZEND_ITER_PLAIN_OBJECT:
1432+
case ZEND_ITER_PLAIN_ARRAY:
1433+
zend_untrack_hash_position(HASH_OF(array), &EX_T(opline->op1.var).fe.fe_pos);
1434+
default:
1435+
break;
1436+
}
1437+
1438+
zval_ptr_dtor(&array);
1439+
}
1440+
break;
1441+
}
1442+
}
1443+
/* }}} */
1444+
1445+
void zend_handle_free_op(zend_op *opline, const zend_execute_data *execute_data TSRMLS_DC) /* {{{ */
1446+
{
1447+
i_handle_free_op(opline, execute_data TSRMLS_CC);
1448+
}
1449+
/* }}} */
1450+
14121451
static inline zend_brk_cont_element* zend_brk_cont(int nest_levels, int array_offset, const zend_op_array *op_array, const zend_execute_data *execute_data TSRMLS_DC)
14131452
{
14141453
int original_nest_levels = nest_levels;
@@ -1420,20 +1459,7 @@ static inline zend_brk_cont_element* zend_brk_cont(int nest_levels, int array_of
14201459
}
14211460
jmp_to = &op_array->brk_cont_array[array_offset];
14221461
if (nest_levels>1) {
1423-
zend_op *brk_opline = &op_array->opcodes[jmp_to->brk];
1424-
1425-
switch (brk_opline->opcode) {
1426-
case ZEND_SWITCH_FREE:
1427-
if (!(brk_opline->extended_value & EXT_TYPE_FREE_ON_RETURN)) {
1428-
zval_ptr_dtor(&EX_T(brk_opline->op1.var).var.ptr);
1429-
}
1430-
break;
1431-
case ZEND_FREE:
1432-
if (!(brk_opline->extended_value & EXT_TYPE_FREE_ON_RETURN)) {
1433-
zendi_zval_dtor(EX_T(brk_opline->op1.var).tmp_var);
1434-
}
1435-
break;
1436-
}
1462+
i_handle_free_op(&op_array->opcodes[jmp_to->brk], execute_data TSRMLS_CC);
14371463
}
14381464
array_offset = jmp_to->parent;
14391465
} while (--nest_levels > 0);

‎Zend/zend_execute.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ typedef union _temp_variable {
4242
struct {
4343
zval **ptr_ptr; /* shared with var.ptr_ptr */
4444
zval *ptr; /* shared with var.ptr */
45-
HashPointer fe_pos;
45+
HashPosition fe_pos;
4646
} fe;
4747
zend_class_entry *class_entry;
4848
} temp_variable;
@@ -393,6 +393,7 @@ ZEND_API zval **zend_get_zval_ptr_ptr(int op_type, const znode_op *node, const z
393393

394394
ZEND_API int zend_do_fcall(ZEND_OPCODE_HANDLER_ARGS);
395395

396+
void zend_handle_free_op(zend_op *opline, const zend_execute_data *execute_data TSRMLS_DC);
396397
void zend_clean_and_cache_symbol_table(HashTable *symbol_table TSRMLS_DC);
397398
void zend_free_compiled_variables(zend_execute_data *execute_data);
398399

‎Zend/zend_generators.c

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -72,22 +72,7 @@ ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished
7272
} else if (brk_cont->start > op_num) {
7373
break;
7474
} else if (brk_cont->brk > op_num) {
75-
zend_op *brk_opline = op_array->opcodes + brk_cont->brk;
76-
77-
switch (brk_opline->opcode) {
78-
case ZEND_SWITCH_FREE:
79-
{
80-
temp_variable *var = EX_TMP_VAR(execute_data, brk_opline->op1.var);
81-
zval_ptr_dtor(&var->var.ptr);
82-
}
83-
break;
84-
case ZEND_FREE:
85-
{
86-
temp_variable *var = EX_TMP_VAR(execute_data, brk_opline->op1.var);
87-
zval_dtor(&var->tmp_var);
88-
}
89-
break;
90-
}
75+
zend_handle_free_op(op_array->opcodes + brk_cont->brk, execute_data TSRMLS_CC);
9176
}
9277
}
9378
}

0 commit comments

Comments
 (0)
Please sign in to comment.