Famous last bug in PolicyArray

PolicyArray pased all tests but failed in production. After browsing on my own box I figured that what happened was the method addLval() with a string key is called in production on many pages, but never in the tests. @ptarjan maybe you could look into adding such a test?

This diff still does not instantiate the policy-based array (I did instantiate it during testing).

hphp/runtime/base/array/policy_array.cpp

@@ -481,7 +481,8 @@ ArrayData *ArrayShell::addImpl(K k, const Variant& v, bool copy) {
   return this;
 }
 
-ArrayShell *ArrayShell::addLval(int64_t k, Variant*& ret, bool copy) {
+template <class K>
+ArrayShell *ArrayShell::addLvalImpl(K k, Variant*& ret, bool copy) {
   APILOG << "(" << k << ", " << ret << ", " << copy << ")";
   if (copy) {
     return ArrayShell::copy()->addLval(k, ret, false);
@@ -495,10 +496,6 @@ ArrayShell *ArrayShell::addLval(int64_t k, Variant*& ret, bool copy) {
   return this;
 }
 
-ArrayShell *ArrayShell::addLval(StringData* k, Variant *&ret, bool copy) {
-  NOT_IMPLEMENTED();
-}
-
 template <class K>
 ArrayData *ArrayShell::removeImpl(K k, bool copy) {
   APILOG << "(" << keystr(k) << ", " << copy << ")";

hphp/runtime/base/array/policy_array.h

@@ -555,10 +555,19 @@ public:
    * Same semantics as lval(), except with the precondition that the
    * key doesn't already exist in the array.
    */
+private:
+  template <class K>
+  ArrayShell* addLvalImpl(K k, Variant*& ret, bool copy);
+
+public:
   virtual ArrayShell *addLval(int64_t k, Variant *&ret, bool copy)
-    FOLLY_OVERRIDE;
+    FOLLY_OVERRIDE {
+    return addLvalImpl(k, ret, copy);
+  }
   virtual ArrayShell *addLval(StringData* k, Variant *&ret, bool copy)
-    FOLLY_OVERRIDE;
+    FOLLY_OVERRIDE {
+    return addLvalImpl(k, ret, copy);
+  }
 
   /**
    * Remove a value at specified key. If "copy" is true, make a copy first

hphp/test/slow/apc/1813.php

@@ -1,6 +1,6 @@
 <?php
 
- $a = array(1);
+$a = array(1);
 $a[] =& $a[0];
 $a[0] = 2;
 print_r($a);
@@ -10,3 +10,15 @@ $b = apc_fetch('table', $b);
 print_r($b);
 $b[0] = 3;
 print_r($b);
+
+$a = array('xyz' => 'tuv');
+$a[] =& $a[0];
+$a[0] = 2;
+print_r($a);
+
+apc_store('table', $a);
+$b = apc_fetch('table', $b);
+print_r($b);
+$b[0] = 3;
+print_r($b);
+

hphp/test/slow/apc/1813.php.expect

@@ -13,3 +13,21 @@ Array
     [0] => 3
     [1] => 3
 )
+Array
+(
+    [xyz] => tuv
+    [0] => 2
+    [1] => 2
+)
+Array
+(
+    [xyz] => tuv
+    [0] => 2
+    [1] => 2
+)
+Array
+(
+    [xyz] => tuv
+    [0] => 3
+    [1] => 3
+)