Merge pull request #799 from capnproto/insertion-order-index-manages-memory-correctly

Implement InsertionOrderIndex move constructor/assignment operator

c++/src/kj/table-test.c++

@@ -1101,6 +1101,66 @@ KJ_TEST("insertion order index") {
   }
 }
 
+KJ_TEST("insertion order index is movable") {
+  using UintTable = Table<uint, InsertionOrderIndex>;
+
+  kj::Maybe<UintTable> myTable;
+
+  {
+    UintTable yourTable;
+
+    yourTable.insert(12);
+    yourTable.insert(34);
+    yourTable.insert(56);
+    yourTable.insert(78);
+    yourTable.insert(111);
+    yourTable.insert(222);
+    yourTable.insert(333);
+    yourTable.insert(444);
+    yourTable.insert(555);
+    yourTable.insert(666);
+    yourTable.insert(777);
+    yourTable.insert(888);
+    yourTable.insert(999);
+
+    myTable = kj::mv(yourTable);
+  }
+
+  auto& table = KJ_ASSERT_NONNULL(myTable);
+
+  // At one time the following induced a segfault/double-free, due to incorrect memory management in
+  // InsertionOrderIndex's move ctor and dtor.
+  auto range = table.ordered();
+  auto iter = range.begin();
+  KJ_ASSERT(iter != range.end());
+  KJ_EXPECT(*iter++ == 12);
+  KJ_ASSERT(iter != range.end());
+  KJ_EXPECT(*iter++ == 34);
+  KJ_ASSERT(iter != range.end());
+  KJ_EXPECT(*iter++ == 56);
+  KJ_ASSERT(iter != range.end());
+  KJ_EXPECT(*iter++ == 78);
+  KJ_ASSERT(iter != range.end());
+  KJ_EXPECT(*iter++ == 111);
+  KJ_ASSERT(iter != range.end());
+  KJ_EXPECT(*iter++ == 222);
+  KJ_ASSERT(iter != range.end());
+  KJ_EXPECT(*iter++ == 333);
+  KJ_ASSERT(iter != range.end());
+  KJ_EXPECT(*iter++ == 444);
+  KJ_ASSERT(iter != range.end());
+  KJ_EXPECT(*iter++ == 555);
+  KJ_ASSERT(iter != range.end());
+  KJ_EXPECT(*iter++ == 666);
+  KJ_ASSERT(iter != range.end());
+  KJ_EXPECT(*iter++ == 777);
+  KJ_ASSERT(iter != range.end());
+  KJ_EXPECT(*iter++ == 888);
+  KJ_ASSERT(iter != range.end());
+  KJ_EXPECT(*iter++ == 999);
+  KJ_EXPECT(iter == range.end());
+}
+
 }  // namespace
 }  // namespace _
 }  // namespace kj

c++/src/kj/table.c++

@@ -808,6 +808,19 @@ void BTreeImpl::Parent::eraseAfter(uint i) {
 const InsertionOrderIndex::Link InsertionOrderIndex::EMPTY_LINK = { 0, 0 };
 
 InsertionOrderIndex::InsertionOrderIndex(): capacity(0), links(const_cast<Link*>(&EMPTY_LINK)) {}
+InsertionOrderIndex::InsertionOrderIndex(InsertionOrderIndex&& other)
+    : capacity(other.capacity), links(other.links) {
+  other.capacity = 0;
+  other.links = const_cast<Link*>(&EMPTY_LINK);
+}
+InsertionOrderIndex& InsertionOrderIndex::operator=(InsertionOrderIndex&& other) {
+  KJ_DASSERT(&other != this);
+  capacity = other.capacity;
+  links = other.links;
+  other.capacity = 0;
+  other.links = const_cast<Link*>(&EMPTY_LINK);
+  return *this;
+}
 InsertionOrderIndex::~InsertionOrderIndex() noexcept(false) {
   if (links != &EMPTY_LINK) delete[] links;
 }

c++/src/kj/table.h

@@ -1538,6 +1538,10 @@ class InsertionOrderIndex {
   struct Link;
 public:
   InsertionOrderIndex();
+  InsertionOrderIndex(const InsertionOrderIndex&) = delete;
+  InsertionOrderIndex& operator=(const InsertionOrderIndex&) = delete;
+  InsertionOrderIndex(InsertionOrderIndex&& other);
+  InsertionOrderIndex& operator=(InsertionOrderIndex&& other);
   ~InsertionOrderIndex() noexcept(false);
 
   class Iterator {