[libcxx-commits] [libcxx] 66a0203 - [libc++] Fix exception safety of `__hash_table::__copy_construct` (avoid memory leak) (#201452)
via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Jun 10 00:06:01 PDT 2026
Author: Nikita Taranov
Date: 2026-06-10T09:05:56+02:00
New Revision: 66a0203502d2105c027b8494f6fd6d22cc9b6cfc
URL: https://github.com/llvm/llvm-project/commit/66a0203502d2105c027b8494f6fd6d22cc9b6cfc
DIFF: https://github.com/llvm/llvm-project/commit/66a0203502d2105c027b8494f6fd6d22cc9b6cfc.diff
LOG: [libc++] Fix exception safety of `__hash_table::__copy_construct` (avoid memory leak) (#201452)
Slightly easier to digest repro: https://godbolt.org/z/ejjs5br5f
Added:
libcxx/test/std/containers/unord/unord.map/unord.map.cnstr/exceptions.pass.cpp
Modified:
libcxx/include/__hash_table
Removed:
################################################################################
diff --git a/libcxx/include/__hash_table b/libcxx/include/__hash_table
index e1264703f6b18..0ce4cf3edd6fd 100644
--- a/libcxx/include/__hash_table
+++ b/libcxx/include/__hash_table
@@ -41,6 +41,7 @@
#include <__type_traits/is_swappable.h>
#include <__type_traits/remove_const.h>
#include <__type_traits/remove_cvref.h>
+#include <__utility/exception_guard.h>
#include <__utility/exchange.h>
#include <__utility/forward.h>
#include <__utility/move.h>
@@ -697,6 +698,17 @@ private:
_LIBCPP_HIDE_FROM_ABI void __copy_construct(__next_pointer __other_iter) {
__next_pointer __own_iter = __first_node_.__ptr();
+ // If copying a node throws, the nodes that have already been constructed and linked into the
+ // list have to be deallocated. When this is called from the copy constructor the enclosing
+ // __hash_table is not fully constructed yet, so its destructor would not run to release them.
+ // This is also reached from operator= when assigning into an empty table; there the table
+ // stays alive, so we must leave it in a valid empty state: besides releasing the nodes we
+ // clear the bucket pointers, which were made to point into the now-deallocated node list.
+ auto __guard = std::__make_exception_guard([&] {
+ __deallocate_node_list(__first_node_.__next_);
+ __first_node_.__next_ = nullptr;
+ std::fill_n(__bucket_list_.get(), bucket_count(), nullptr);
+ });
{
__node_holder __new_node = __construct_node_hash(__other_iter->__hash(), __other_iter->__upcast()->__get_value());
__own_iter->__next_ = static_cast<__next_pointer>(__new_node.release());
@@ -707,6 +719,7 @@ private:
__other_iter = __other_iter->__next_;
__own_iter = __own_iter->__next_;
__copy_construct(__other_iter, __own_iter, __current_chash);
+ __guard.__complete();
}
public:
diff --git a/libcxx/test/std/containers/unord/unord.map/unord.map.cnstr/exceptions.pass.cpp b/libcxx/test/std/containers/unord/unord.map/unord.map.cnstr/exceptions.pass.cpp
new file mode 100644
index 0000000000000..f2aff91caa446
--- /dev/null
+++ b/libcxx/test/std/containers/unord/unord.map/unord.map.cnstr/exceptions.pass.cpp
@@ -0,0 +1,121 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: no-exceptions
+
+// <unordered_map>
+
+// Check that the unordered_map copy constructor does not leak nodes when a node allocation
+// throws partway through copying the elements. The already-copied nodes must be released even
+// though the container itself never finishes constructing.
+//
+// Also check the copy-assignment-into-empty path, which shares the same node-copying helper.
+// There the assigned-to container stays alive, so after a throw it must be left in a valid
+// (usable, leak-free) state with no dangling bucket pointers.
+
+#include <cassert>
+#include <cstddef>
+#include <functional>
+#include <memory>
+#include <new>
+#include <unordered_map>
+#include <utility>
+
+#include "count_new.h"
+#include "test_macros.h"
+
+// An allocator that throws std::bad_alloc once a shared countdown reaches zero. It forwards
+// to std::allocator (operator new) so that count_new.h can observe leaked nodes.
+template <class T>
+struct CountedThrowingAllocator {
+ typedef T value_type;
+
+ int* countdown_;
+
+ explicit CountedThrowingAllocator(int& countdown) : countdown_(&countdown) {}
+
+ template <class U>
+ CountedThrowingAllocator(const CountedThrowingAllocator<U>& other) : countdown_(other.countdown_) {}
+
+ T* allocate(std::size_t n) {
+ if (*countdown_ == 0)
+ throw std::bad_alloc();
+ --*countdown_;
+ return std::allocator<T>().allocate(n);
+ }
+
+ void deallocate(T* p, std::size_t n) { std::allocator<T>().deallocate(p, n); }
+
+ template <class U>
+ friend bool operator==(const CountedThrowingAllocator&, const CountedThrowingAllocator<U>&) {
+ return true;
+ }
+
+ template <class U>
+ friend bool operator!=(const CountedThrowingAllocator&, const CountedThrowingAllocator<U>&) {
+ return false;
+ }
+};
+
+int main(int, char**) {
+ typedef CountedThrowingAllocator<std::pair<const int, int> > Alloc;
+ typedef std::unordered_map<int, int, std::hash<int>, std::equal_to<int>, Alloc> Map;
+
+ const int never_throw = 1 << 30;
+ int countdown = never_throw;
+
+ // Build a source map large enough that copying it performs several node allocations. The
+ // allocator is propagated by select_on_container_copy_construction, so the copy shares this
+ // same countdown.
+ Map src((Alloc(countdown)));
+ for (int i = 0; i < 16; ++i)
+ src.insert(std::make_pair(i, i));
+
+ // Make a node allocation fail at every position partway through the copy and verify that no
+ // nodes are leaked. `outstanding_before` captures the allocations held by the source so the
+ // check is unaffected by it.
+ for (int fail_at = 0; fail_at < 32; ++fail_at) {
+ const int outstanding_before = globalMemCounter.outstanding_new;
+
+ countdown = fail_at;
+ try {
+ Map copy(src);
+ (void)copy;
+ } catch (const std::bad_alloc&) {
+ }
+ countdown = never_throw;
+
+ assert(globalMemCounter.outstanding_new == outstanding_before);
+ }
+
+ // Same, but through copy-assignment into an empty (default-constructed) map. The destination
+ // survives the exception, so it must be left valid: we exercise it with insert()/find() (which
+ // read the bucket array and would dereference a stale entry under a sanitizer) and confirm no
+ // nodes are leaked once it goes out of scope.
+ for (int fail_at = 0; fail_at < 32; ++fail_at) {
+ const int outstanding_before = globalMemCounter.outstanding_new;
+ {
+ Map dst((Alloc(countdown)));
+ countdown = fail_at;
+ try {
+ dst = src;
+ } catch (const std::bad_alloc&) {
+ }
+ countdown = never_throw;
+
+ // The container must be in a valid state whether or not the assignment threw.
+ for (int i = 0; i < 16; ++i)
+ dst.insert(std::make_pair(i, i));
+ for (int i = 0; i < 16; ++i)
+ assert(dst.find(i) != dst.end());
+ }
+ assert(globalMemCounter.outstanding_new == outstanding_before);
+ }
+
+ return 0;
+}
More information about the libcxx-commits
mailing list