Merge pull request #53573 from Klowner/53568-safelist-leak-fix

This commit is contained in:
Rémi Verschelde 2022-04-11 17:55:35 +02:00 committed by GitHub
commit f62f36cd91
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 26 additions and 4 deletions

View File

@ -203,7 +203,7 @@ public:
} }
// Calling this will cause zero to many deallocations. // Calling this will cause zero to many deallocations.
void maybe_cleanup() { bool maybe_cleanup() {
SafeListNode *cursor = nullptr; SafeListNode *cursor = nullptr;
SafeListNode *new_graveyard_head = nullptr; SafeListNode *new_graveyard_head = nullptr;
do { do {
@ -212,7 +212,7 @@ public:
if (active_iterator_count.load() != 0) { if (active_iterator_count.load() != 0) {
// It's not safe to clean up with an active iterator, because that iterator // It's not safe to clean up with an active iterator, because that iterator
// could be pointing to an element that we want to delete. // could be pointing to an element that we want to delete.
return; return false;
} }
// Any iterator created after this point will never point to a deleted node. // Any iterator created after this point will never point to a deleted node.
// Swap it out with the current graveyard head. // Swap it out with the current graveyard head.
@ -225,6 +225,17 @@ public:
tmp->deletion_fn(tmp->val); tmp->deletion_fn(tmp->val);
memdelete_allocator<SafeListNode, A>(tmp); memdelete_allocator<SafeListNode, A>(tmp);
} }
return true;
}
~SafeList() {
#ifdef DEBUG_ENABLED
if (!maybe_cleanup()) {
ERR_PRINT("There are still iterators around when destructing a SafeList. Memory will be leaked. This is a bug.");
}
#else
maybe_cleanup();
#endif
} }
}; };
@ -353,11 +364,11 @@ public:
} }
// Calling this will cause zero to many deallocations. // Calling this will cause zero to many deallocations.
void maybe_cleanup() { bool maybe_cleanup() {
SafeListNode *cursor = graveyard_head; SafeListNode *cursor = graveyard_head;
if (active_iterator_count != 0) { if (active_iterator_count != 0) {
// It's not safe to clean up with an active iterator, because that iterator could be pointing to an element that we want to delete. // It's not safe to clean up with an active iterator, because that iterator could be pointing to an element that we want to delete.
return; return false;
} }
graveyard_head = nullptr; graveyard_head = nullptr;
// Our graveyard list is now unreachable by any active iterators, detached from the main graveyard head and ready for deletion. // Our graveyard list is now unreachable by any active iterators, detached from the main graveyard head and ready for deletion.
@ -367,6 +378,17 @@ public:
tmp->deletion_fn(tmp->val); tmp->deletion_fn(tmp->val);
memdelete_allocator<SafeListNode, A>(tmp); memdelete_allocator<SafeListNode, A>(tmp);
} }
return true;
}
~SafeList() {
#ifdef DEBUG_ENABLED
if (!maybe_cleanup()) {
ERR_PRINT("There are still iterators around when destructing a SafeList. Memory will be leaked. This is a bug.");
}
#else
maybe_cleanup();
#endif
} }
}; };