Phalanger icon indicating copy to clipboard operation
Phalanger copied to clipboard

bug with references and array's copy on write

Open proff opened this issue 11 years ago • 7 comments

<?
function test()
{
       global $a;

       $b = &$a[0];
       $b = 1;
}

$a = array(0);
test();
$d = $a;
$a[0] = 2;
echo $d[0];
?>

in php result is 1, in phalanger 2

proff avatar Sep 16 '13 15:09 proff

        private void PHP.Core.OrderedDictionary._deep_copy_entry_value(object oldref, object newref, ref Entry entry)
        {
            var r = entry.Value as PhpReference;
            if (r != null)
            {
                if (r.Value == oldref)
                    entry.Value = new PhpReference(newref);
                else
                    entry.Value = new PhpReference(r.Value);
            }
            else
                entry.Value = PhpVariable.DeepCopy(entry.Value);
        }

but i'm not tested for new bugs yet

proff avatar Sep 16 '13 18:09 proff

This change is equal the preview version new PhpReference(r.Value) === PhpVariable.DeepCopy(entry.Value) // in case entry.Value is PhpReference, see PhpReference.DeepCopy()

jakubmisek avatar Sep 18 '13 14:09 jakubmisek

new PhpReference(r.Value) !== PhpVariable.DeepCopy(entry.Value) in left part it will be new instance of the PhpReference, in right part it will be same instance of the PhpReference (and "$a[0] = 2;" line change value in both arrays because theirs contains same instance of the PhpReference).

proff avatar Sep 18 '13 14:09 proff

I see, right

jakubmisek avatar Sep 18 '13 14:09 jakubmisek

So $b = &$a[0] // upgrades 0th element to PhpRreference $d = $a; // increases copy count of internal PhpHashtable (CopyOnWrite mechanism) $a[0] = 2; // starts CopyOnWrite ... DeepCopy all elements, how do we handle PhpReference?

  1. references to self should be changed to new self (there is bug too, if we are lazy-copying on original self, the new self would have copy count back to 1 and reference to self of another PhpHashtable instance won't be updated)
  2. other references are needed? Shouldn't we dereference all the values ?

jakubmisek avatar Sep 18 '13 21:09 jakubmisek

If we create new PhpReference instance, following test would fails I guess:

<?php

function test()
{
    $x = 1;
    $arr = [&$x];
    $arr[0] = 2;
    var_dump($x, $arr);

    $arr2 = $arr;
    $arr2[0] = 3;
    var_dump($x, $arr, $arr2);
}
test();

jakubmisek avatar Oct 03 '13 16:10 jakubmisek

AH! I think the problem is somewhere else ...

PHP COUNTS references. As $b dies, $a[0] behaves as a value type again.

We've encountered this issue earlier, and it would require to implement own garbage collector I afraid.

jakubmisek avatar Oct 03 '13 17:10 jakubmisek