FW: memory corruptions in APC env when using references or extract(..., EXTR_REFS) [was re: references / zend_assign_to_variable / IS_CONST operands in APC] View as Markdown

Resending the mail since I didn't get any responses.

To add:

This seems to be a fundamental issue with running PHP in an environment
that keeps op arrays in shared memory (such as APC) and using
"references" or other reference semantic features of PHP (such as
extract with the EXTR_REFS option).

Temporarily, we are working around it by setting tweaking
zend_opcode.c:pass_two(). Recall that pass_two() sets is_ref to 1 and
refcount to 2 for all IS_CONST operands in the op arrays.

Existing code from zend_opcode.c:pass_two():

	if (opline->op1.op_type == IS_CONST) {
		opline->op1.u.constant.is_ref = 1;
		opline->op1.u.constant.refcount = 2; /* Make

sure is_ref won't be reset */
}
if (opline->op2.op_type == IS_CONST) {
opline->op2.u.constant.is_ref = 1;
opline->op2.u.constant.refcount = 2;
}

To minimize the chance of race condition I describe in the earlier mail,
we are setting refcount in both cases above to a much larger value
(~100M) instead of 2; and have stopped seeing problems now. But this is
a workaround at best.

Would like feedback on whether my analysis is correct in the first
place, and are if there any thoughts on a better fix.

regards,
Kannan
-----Original Message-----
From: Kannan Muthukkaruppan [mailto:Kannan@facebook.com]
Sent: Friday, March 28, 2008 7:06 PM
To: internals@lists.php.net
Subject: [PHP-DEV] references / zend_assign_to_variable / IS_CONST
operands in APC

Consider the following small test case where $y is first made to be a
reference to $x. And next, we assign a literal to $y.

Example #1:
<?php

$x = 5;
$y = &$x;

$y = null; <---

When we come to the third assignment statement above we try to increase
(followed quickly by a decrease) of the "refcount" on the right hand
side ZVAL (the null zval) in the following piece of code in
zend_assign_to_variable():

} else if (PZVAL_IS_REF(variable_ptr)) {
if (variable_ptr!=value) {
zend_uint refcount = variable_ptr->refcount;
zval garbage;

if (type!=IS_TMP_VAR) {
value->refcount++; <----- incrementing the refcount
of RHS zval
}
garbage = *variable_ptr;
*variable_ptr = *value;
variable_ptr->refcount = refcount;
variable_ptr->is_ref = 1;
if (type!=IS_TMP_VAR) {
zendi_zval_copy_ctor(*variable_ptr);
value->refcount--; <------ and then decrementing
it back...
}
zendi_zval_dtor(garbage);
}
}

I am trying to understand the rationale for value->refcount++ and
value->refcount-- above. Why do we need that?

With PHP running in apache+APC environment, the "null" literal is an
IS_CONST operand in the op_array in APC (shared memory). And
incrementing/decrementing a ref count on the shared memory area without
a mutex can cause unexpected problems due to race conditions (and
eventually lead to memory corruptions etc.)

Recall that zend_opcode.c:pass_two() sets "is_ref" on all IS_CONST
operands to 1, as a way to allow op_arrays to be sharable across
processes. And indeed, in the simpler case of an assignment such as in
the following example (#2), zend_assign_to_variable() notices that the
RHS has "is_ref" set and goes on to make a copy (rather than
incrementing the RHS's refcount).

Example #2:

<?php
$a = 5;

But in the case the LHS is itself an "is_ref" (like in the example #1)
it doesn't seem to make a similar check on the RHS (to see if it is safe
to modify the refcount on the RHS). Why?

regards,
Kannan