Make SHOW_MEMORY builds work: fix deferred node capacity assert and eraseTree use-after-free #91
+51
-27
@@ -2757,31 +2757,53 @@ checkMaxBetweenExclusiveImpl<true>(Node256 *n, int begin, int end,
|
|||||||
return self;
|
return self;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#if SHOW_MEMORY
|
||||||
|
// Length of the key leading to `child`, given the length of the key leading to
|
||||||
|
// its parent. Consistent with `getSearchPathLength`.
|
||||||
|
int64_t childSearchPathLength(int64_t parentSearchPathLength, Node *child) {
|
||||||
|
return parentSearchPathLength + 1 + child->partialKeyLen;
|
||||||
|
}
|
||||||
|
#endif
|
||||||
|
|
||||||
|
|
|||||||
void eraseTree(Node *root, WriteContext *writeContext) {
|
void eraseTree(Node *root, WriteContext *writeContext) {
|
||||||
// A node is released before its children are visited, so the `parent`
|
|
||||||
// pointers of queued nodes can't be relied upon. Carry the search path length
|
|
||||||
// of each node along with it instead.
|
|
||||||
struct ToFree {
|
|
||||||
Node *n;
|
|
||||||
// Length of the key leading to `n`. See `getSearchPathLength`.
|
|
||||||
int64_t searchPathLength;
|
|
||||||
};
|
|
||||||
Arena arena;
|
Arena arena;
|
||||||
auto toFree = vector<ToFree>(arena);
|
auto toFree = vector<Node *>(arena);
|
||||||
toFree.push_back({root, getSearchPathLength(root)});
|
toFree.push_back(root);
|
||||||
|
// Length of the key leading to each node in `toFree`, on a parallel stack.
|
||||||
|
// A node is released before its children are visited, so the `parent`
|
||||||
|
// pointers that `getSearchPathLength` walks are dangling by the time a queued
|
||||||
|
// node is popped. Carry the lengths along instead. Only the SHOW_MEMORY
|
||||||
|
// accounting needs them.
|
||||||
|
#if SHOW_MEMORY
|
||||||
|
auto searchPathLengths = vector<int64_t>(arena);
|
||||||
|
searchPathLengths.push_back(getSearchPathLength(root));
|
||||||
|
#endif
|
||||||
|
|
||||||
|
// Queue `child` for release, where `parentSearchPathLength` is the length of
|
||||||
|
// the key leading to its parent.
|
||||||
|
auto queueChild = [&](Node *child, int64_t parentSearchPathLength) {
|
||||||
|
toFree.push_back(child);
|
||||||
|
#if SHOW_MEMORY
|
||||||
|
searchPathLengths.push_back(
|
||||||
|
childSearchPathLength(parentSearchPathLength, child));
|
||||||
|
#else
|
||||||
|
(void)parentSearchPathLength;
|
||||||
|
#endif
|
||||||
|
};
|
||||||
|
|
||||||
while (toFree.size() > 0) {
|
while (toFree.size() > 0) {
|
||||||
const auto item = toFree.back();
|
auto *n = toFree.back();
|
||||||
auto *n = item.n;
|
|
||||||
toFree.pop_back();
|
toFree.pop_back();
|
||||||
writeContext->accum.entries_erased += n->entryPresent;
|
writeContext->accum.entries_erased += n->entryPresent;
|
||||||
++writeContext->accum.nodes_released;
|
++writeContext->accum.nodes_released;
|
||||||
|
|
||||||
removeKey(n, item.searchPathLength);
|
#if SHOW_MEMORY
|
||||||
|
const int64_t searchPathLength = searchPathLengths.back();
|
||||||
// Search path length of the children of `n`. Must be read before `n` is
|
searchPathLengths.pop_back();
|
||||||
// released.
|
#else
|
||||||
const int64_t childSearchPathLengthBase = item.searchPathLength + 1;
|
const int64_t searchPathLength = 0;
|
||||||
|
#endif
|
||||||
|
removeKey(n, searchPathLength);
|
||||||
|
|
||||||
switch (n->getType()) {
|
switch (n->getType()) {
|
||||||
case Type_Node0: {
|
case Type_Node0: {
|
||||||
@@ -2791,36 +2813,38 @@ void eraseTree(Node *root, WriteContext *writeContext) {
|
|||||||
case Type_Node3: {
|
case Type_Node3: {
|
||||||
auto *n3 = static_cast<Node3 *>(n);
|
auto *n3 = static_cast<Node3 *>(n);
|
||||||
for (int i = 0; i < n3->numChildren; ++i) {
|
for (int i = 0; i < n3->numChildren; ++i) {
|
||||||
Node *child = n3->children[i];
|
queueChild(n3->children[i], searchPathLength);
|
||||||
toFree.push_back(
|
|
||||||
{child, childSearchPathLengthBase + child->partialKeyLen});
|
|
||||||
}
|
}
|
||||||
writeContext->release(n3);
|
writeContext->release(n3);
|
||||||
} break;
|
} break;
|
||||||
case Type_Node16: {
|
case Type_Node16: {
|
||||||
auto *n16 = static_cast<Node16 *>(n);
|
auto *n16 = static_cast<Node16 *>(n);
|
||||||
for (int i = 0; i < n16->numChildren; ++i) {
|
for (int i = 0; i < n16->numChildren; ++i) {
|
||||||
Node *child = n16->children[i];
|
queueChild(n16->children[i], searchPathLength);
|
||||||
toFree.push_back(
|
|
||||||
{child, childSearchPathLengthBase + child->partialKeyLen});
|
|
||||||
}
|
}
|
||||||
writeContext->release(n16);
|
writeContext->release(n16);
|
||||||
} break;
|
} break;
|
||||||
case Type_Node48: {
|
case Type_Node48: {
|
||||||
auto *n48 = static_cast<Node48 *>(n);
|
auto *n48 = static_cast<Node48 *>(n);
|
||||||
for (int i = 0; i < n48->numChildren; ++i) {
|
for (int i = 0; i < n48->numChildren; ++i) {
|
||||||
Node *child = n48->children[i];
|
queueChild(n48->children[i], searchPathLength);
|
||||||
toFree.push_back(
|
|
||||||
{child, childSearchPathLengthBase + child->partialKeyLen});
|
|
||||||
}
|
}
|
||||||
writeContext->release(n48);
|
writeContext->release(n48);
|
||||||
} break;
|
} break;
|
||||||
case Type_Node256: {
|
case Type_Node256: {
|
||||||
auto *n256 = static_cast<Node256 *>(n);
|
auto *n256 = static_cast<Node256 *>(n);
|
||||||
auto *out = toFree.unsafePrepareAppend(n256->numChildren).data();
|
auto *out = toFree.unsafePrepareAppend(n256->numChildren).data();
|
||||||
|
#if SHOW_MEMORY
|
||||||
|
auto *outSearchPathLengths =
|
||||||
|
searchPathLengths.unsafePrepareAppend(n256->numChildren).data();
|
||||||
|
#endif
|
||||||
n256->bitSet.forEachSet([&](int i) {
|
n256->bitSet.forEachSet([&](int i) {
|
||||||
Node *child = n256->children[i];
|
Node *child = n256->children[i];
|
||||||
*out++ = {child, childSearchPathLengthBase + child->partialKeyLen};
|
*out++ = child;
|
||||||
|
#if SHOW_MEMORY
|
||||||
|
*outSearchPathLengths++ =
|
||||||
|
childSearchPathLength(searchPathLength, child);
|
||||||
|
#endif
|
||||||
});
|
});
|
||||||
assert(out == toFree.end());
|
assert(out == toFree.end());
|
||||||
writeContext->release(n256);
|
writeContext->release(n256);
|
||||||
|
|||||||
Reference in new issue
Block a user
Can we only include searchPathLength when SHOW_MEMORY=1 ?