Fixed a bug in cdhashmap in which doubly-linked list was not properly cleaned up...

Also, removed some experimental code and a unit test from cdmap_black that used it. This test created a CDList *in* context memory which seems like a very bad idea (and
it was improperly implemented resulting in a memory leak).

diff --git a/src/context/cdhashmap.h b/src/context/cdhashmap.h
index 6ae74fbde94689d20e8e40c5133558c585c09406..0a6c400a79c7568d09cc225768659dca9c42c4c1 100644 (file)
--- a/src/context/cdhashmap.h
+++ b/src/context/cdhashmap.h
@@ -163,7 +163,7 @@ class CDOhash_map : public ContextObj {
         d_data = p->d_data;
       }
     }
-    // Explicitly call destructors for the key and the date as they will not
+    // Explicitly call destructors for the key and the data as they will not
     // otherwise get called.
     p->d_key.~Key();
     p->d_data.~Data();
@@ -418,21 +418,6 @@ public:
     }
   }
 
-  // Use this for pointer data d allocated in context memory at this
-  // level.  THIS IS HIGHLY EXPERIMENTAL.  It seems to work if ALL
-  // your data objects are allocated from context memory.
-  void insertDataFromContextMemory(const Key& k, const Data& d) {
-    emptyTrash();
-
-    AlwaysAssert(d_map.find(k) == d_map.end());
-
-    Element* obj = new(d_context->getCMM()) Element(d_context, this, k, d,
-                                                    false /* atLevelZero */,
-                                                    true /* allocatedInCMM */);
-
-    d_map[k] = obj;
-  }
-
   /**
    * Version of insert() for CDHashMap<> that inserts data value d at
    * context level zero.  This is a special escape hatch for inserting
@@ -494,10 +479,9 @@ public:
           } else {
             d_first = elt->d_next;
           }
-        } else {
-          elt->d_prev->d_next = elt->d_next;
-          elt->d_next->d_prev = elt->d_prev;
         }
+        elt->d_prev->d_next = elt->d_next;
+        elt->d_next->d_prev = elt->d_prev;
         d_map.erase(j);//FIXME multithreading
         Debug("gc") << "key " << k << " obliterated zero-scope: " << elt << std::endl;
         if(!elt->d_noTrash) {

diff --git a/src/context/cdlist.h b/src/context/cdlist.h
index 4a5ebfd302c3f0fce1204d5f208a439660477ac6..47667cc8d1276c2fd464b41ac19df5db4478a1e3 100644 (file)
--- a/src/context/cdlist.h
+++ b/src/context/cdlist.h
@@ -237,23 +237,6 @@ public:
     d_allocator(alloc) {
   }
 
-  /**
-   * Main constructor: d_list starts as NULL, size is 0
-   */
-  CDList(bool allocatedInCMM,
-            Context* context,
-            bool callDestructor = true,
-            const CleanUp& cleanup = CleanUp(),
-            const Allocator& alloc = Allocator()) :
-    ContextObj(allocatedInCMM, context),
-    d_list(NULL),
-    d_size(0),
-    d_callDestructor(callDestructor),
-    d_sizeAlloc(0),
-    d_cleanUp(cleanup),
-    d_allocator(alloc) {
-  }
-
   /**
    * Destructor: delete the list
    */

diff --git a/test/unit/context/cdmap_black.h b/test/unit/context/cdmap_black.h
index 3da4dc0b086039293cd7c3f5246651ff754a5510..c00b7c4d424cc7a126e87a157b66c52c6805dc1d 100644 (file)
--- a/test/unit/context/cdmap_black.h
+++ b/test/unit/context/cdmap_black.h
@@ -914,151 +914,4 @@ public:
     TS_ASSERT(map.find(23) == map.end());
     TS_ASSERT(map[3] == 4);
   }
-
-  struct int_hasher {
-    size_t operator()(int i) const { return i; }
-  };
-
-  struct myint {
-    int x;
-    myint(int i) : x(i) {}
-    ~myint() { std::cout << "dtor " << x << std::endl; }
-    myint& operator=(int i) { x = i; return *this; }
-    bool operator==(int i) const { return x == i; }
-  };
-
-  void testMapOfLists() {
-    //Debug.on("gc");
-    //Debug.on("context");
-
-    CDHashMap<int, CDList<myint>*, int_hasher> map(d_context);
-
-    CDList<myint> *list1, *list2, *list3, *list4;
-
-    TS_ASSERT(map.find(1) == map.end());
-    TS_ASSERT(map.find(2) == map.end());
-    TS_ASSERT(map.find(3) == map.end());
-    TS_ASSERT(map.find(4) == map.end());
-
-    {
-      d_context->push();
-
-      int* x CVC4_UNUSED = (int*) d_context->getCMM()->newData(sizeof(int));
-
-      list1 = new(d_context->getCMM()) CDList<myint>(true, d_context);
-      list2 = new(d_context->getCMM()) CDList<myint>(true, d_context);
-      list3 = new(d_context->getCMM()) CDList<myint>(true, d_context);
-      list4 = new(d_context->getCMM()) CDList<myint>(true, d_context);
-
-      list1->push_back(1);
-      list2->push_back(2);
-      list3->push_back(3);
-      list4->push_back(4);
-
-      map.insertDataFromContextMemory(1, list1);
-      map.insertDataFromContextMemory(2, list2);
-
-      {
-        d_context->push();
-
-        list1->push_back(10);
-        list2->push_back(20);
-        list3->push_back(30);
-        list4->push_back(40);
-
-        map.insertDataFromContextMemory(3, list3);
-        map.insertDataFromContextMemory(4, list4);
-
-        x = (int*) d_context->getCMM()->newData(sizeof(int));
-
-        TS_ASSERT(map.find(1) != map.end());
-        TS_ASSERT(map.find(2) != map.end());
-        TS_ASSERT(map.find(3) != map.end());
-        TS_ASSERT(map.find(4) != map.end());
-
-        TS_ASSERT(map[1] == list1);
-        TS_ASSERT(map[2] == list2);
-        TS_ASSERT(map[3] == list3);
-        TS_ASSERT(map[4] == list4);
-
-        TS_ASSERT((*list1)[0] == 1);
-        TS_ASSERT((*list2)[0] == 2);
-        TS_ASSERT((*list3)[0] == 3);
-        TS_ASSERT((*list4)[0] == 4);
-
-        TS_ASSERT((*list1)[1] == 10);
-        TS_ASSERT((*list2)[1] == 20);
-        TS_ASSERT((*list3)[1] == 30);
-        TS_ASSERT((*list4)[1] == 40);
-
-        TS_ASSERT(list1->size() == 2);
-        TS_ASSERT(list2->size() == 2);
-        TS_ASSERT(list3->size() == 2);
-        TS_ASSERT(list4->size() == 2);
-
-        d_context->pop();
-      }
-
-      TS_ASSERT(map.find(1) != map.end());
-      TS_ASSERT(map.find(2) != map.end());
-      TS_ASSERT(map.find(3) == map.end());
-      TS_ASSERT(map.find(4) == map.end());
-
-      TS_ASSERT(map[1] == list1);
-      TS_ASSERT(map[2] == list2);
-
-      TS_ASSERT((*list1)[0] == 1);
-      TS_ASSERT((*list2)[0] == 2);
-      TS_ASSERT((*list3)[0] == 3);
-      TS_ASSERT((*list4)[0] == 4);
-
-      TS_ASSERT(list1->size() == 1);
-      TS_ASSERT(list2->size() == 1);
-      TS_ASSERT(list3->size() == 1);
-      TS_ASSERT(list4->size() == 1);
-
-      d_context->pop();
-    }
-
-    {
-      d_context->push();
-
-      // This re-uses context memory used above.  the map.clear()
-      // triggers an emptyTrash() which fails if the CDOhash_maps are put
-      // in the trash.  (We use insertDataFromContextMemory() above to
-      // keep them out of the trash.)
-      cout << "allocating\n";
-      int* x = (int*) d_context->getCMM()->newData(sizeof(int));
-      cout << "x == " << x << std::endl;
-      for(int i = 0; i < 1000; ++i) {
-        *(int*)d_context->getCMM()->newData(sizeof(int)) = 512;
-      }
-      x = (int*) d_context->getCMM()->newData(sizeof(int));
-      cout << "x == " << x << std::endl;
-
-      TS_ASSERT(map.find(1) == map.end());
-      TS_ASSERT(map.find(2) == map.end());
-      TS_ASSERT(map.find(3) == map.end());
-      TS_ASSERT(map.find(4) == map.end());
-
-      map.clear();
-
-      TS_ASSERT(map.find(1) == map.end());
-      TS_ASSERT(map.find(2) == map.end());
-      TS_ASSERT(map.find(3) == map.end());
-      TS_ASSERT(map.find(4) == map.end());
-
-      d_context->pop();
-    }
-
-    TS_ASSERT(d_context->getLevel() == 0);
-  }
-
-  void testCmmElementsAtLevel0() {
-    // this was crashing
-
-    CDHashMap<int, int*, int_hasher> map(d_context);
-    int* a = (int*)d_context->getCMM()->newData(sizeof(int));
-    map.insertDataFromContextMemory(1, a);
-  }
 };