Skip to content

Commit

Permalink
fix #79: don't reuse m_buckets when allocator does not compare equal
Browse files Browse the repository at this point in the history
m_buckets can only be reused when the allocators are equal. Otherwise we would deallocate with m_buckets with the incorrect allocator.

Also bumped the version for the fix.
  • Loading branch information
martinus committed Jul 13, 2023
1 parent e88dd1c commit a9893b3
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 12 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,4 @@ jobs:
name: Windows_Meson_Testlog
path: |
builddir/meson-logs/testlog.txt
builddir/test/udm.exe
builddir/test/udm-test.exe
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
cmake_minimum_required(VERSION 3.12)
project("unordered_dense"
VERSION 4.0.1
VERSION 4.0.2
DESCRIPTION "A fast & densely stored hashmap and hashset based on robin-hood backward shift deletion"
HOMEPAGE_URL "https://github.com/martinus/unordered_dense")

Expand Down
15 changes: 11 additions & 4 deletions include/ankerl/unordered_dense.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
///////////////////////// ankerl::unordered_dense::{map, set} /////////////////////////

// A fast & densely stored hashmap and hashset based on robin-hood backward shift deletion.
// Version 4.0.1
// Version 4.0.2
// https://github.com/martinus/unordered_dense
//
// Licensed under the MIT License <http://opensource.org/licenses/MIT>.
Expand Down Expand Up @@ -32,7 +32,7 @@
// see https://semver.org/spec/v2.0.0.html
#define ANKERL_UNORDERED_DENSE_VERSION_MAJOR 4 // NOLINT(cppcoreguidelines-macro-usage) incompatible API changes
#define ANKERL_UNORDERED_DENSE_VERSION_MINOR 0 // NOLINT(cppcoreguidelines-macro-usage) backwards compatible functionality
#define ANKERL_UNORDERED_DENSE_VERSION_PATCH 1 // NOLINT(cppcoreguidelines-macro-usage) backwards compatible bug fixes
#define ANKERL_UNORDERED_DENSE_VERSION_PATCH 2 // NOLINT(cppcoreguidelines-macro-usage) backwards compatible bug fixes

// API versioning with inline namespace, see https://www.foonathan.net/2018/11/inline-namespaces/

Expand Down Expand Up @@ -906,8 +906,8 @@ class table : public std::conditional_t<is_map_v<T>, base_table_type_map<T>, bas
auto ba = bucket_alloc(m_values.get_allocator());
if (nullptr != m_buckets) {
bucket_alloc_traits::deallocate(ba, m_buckets, bucket_count());
m_buckets = nullptr;
}
m_buckets = nullptr;
m_num_buckets = 0;
m_max_bucket_capacity = 0;
}
Expand Down Expand Up @@ -1221,7 +1221,14 @@ class table : public std::conditional_t<is_map_v<T>, base_table_type_map<T>, bas
if (&other != this) {
deallocate_buckets(); // deallocate before m_values is set (might have another allocator)
m_values = std::move(other.m_values);
m_buckets = std::exchange(other.m_buckets, nullptr);

// we can only reuse m_buckets over when both maps have the same allocator!
if (get_allocator() == other.get_allocator()) {
m_buckets = std::exchange(other.m_buckets, nullptr);
} else {
copy_buckets(other);
}

m_num_buckets = std::exchange(other.m_num_buckets, 0);
m_max_bucket_capacity = std::exchange(other.m_max_bucket_capacity, 0);
m_max_load_factor = std::exchange(other.m_max_load_factor, default_max_load_factor);
Expand Down
2 changes: 1 addition & 1 deletion meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#

project('unordered_dense', 'cpp',
version: '4.0.1',
version: '4.0.2',
license: 'MIT',
default_options : [
'cpp_std=c++17',
Expand Down
2 changes: 1 addition & 1 deletion test/app/stacktrace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ void handle(int sig) {

// print out all the frames to stderr
fmt::print(stderr, "Error: signal {}. See stacktrace with\n", sig);
fmt::print(stderr, "addr2line -Cafpie ./test/udm");
fmt::print(stderr, "addr2line -Cafpie ./test/udm-test");
for (size_t i = 0; i < static_cast<size_t>(size); ++i) {
fmt::print(stderr, " {}", ary[i]);
}
Expand Down
3 changes: 2 additions & 1 deletion test/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ test_sources = [
'unit/namespace.cpp',
'unit/not_copyable.cpp',
'unit/not_moveable.cpp',
'unit/pmr_move_with_allocators.cpp',
'unit/pmr.cpp',
'unit/rehash.cpp',
'unit/replace.cpp',
Expand Down Expand Up @@ -141,7 +142,7 @@ endif
#endif

test_exe = executable(
'udm',
'udm-test',
test_sources,
include_directories: incdir,
cpp_args: cpp_args,
Expand Down
2 changes: 1 addition & 1 deletion test/unit/namespace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

#include <doctest.h>

namespace versioned_namespace = ankerl::unordered_dense::v4_0_1;
namespace versioned_namespace = ankerl::unordered_dense::v4_0_2;

static_assert(std::is_same_v<versioned_namespace::map<int, int>, ankerl::unordered_dense::map<int, int>>);
static_assert(std::is_same_v<versioned_namespace::hash<int>, ankerl::unordered_dense::hash<int>>);
Expand Down
4 changes: 2 additions & 2 deletions test/unit/pmr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,9 @@ TEST_CASE("pmr_move_different_mr") {
show(mr1, "mr1");
show(mr2, "mr2");

REQUIRE(mr1.num_allocs() == 2);
REQUIRE(mr1.num_allocs() == 3);
REQUIRE(mr1.num_deallocs() == 1);
REQUIRE(mr1.num_is_equals() == 0);
REQUIRE(mr1.num_is_equals() == 1);

REQUIRE(mr2.num_allocs() == 2);
REQUIRE(mr2.num_deallocs() == 0);
Expand Down
34 changes: 34 additions & 0 deletions test/unit/pmr_move_with_allocators.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#include <ankerl/unordered_dense.h>

#define ENABLE_LOG_LINE
#include <app/doctest.h>
#include <app/print.h>

#if defined(ANKERL_UNORDERED_DENSE_PMR)
// windows' vector has different allocation behavior, macos has linker errors
# if __linux__

using int_str_map = ankerl::unordered_dense::pmr::map<int, std::string>;

// creates a map and moves it out
auto return_hello_world(ANKERL_UNORDERED_DENSE_PMR::memory_resource* resource) {
int_str_map map_default_resource(resource);
map_default_resource[0] = "Hello";
map_default_resource[1] = "World";
return map_default_resource;
}

TEST_CASE("move_with_allocators") {
int_str_map map_default_resource(ANKERL_UNORDERED_DENSE_PMR::new_delete_resource());

ANKERL_UNORDERED_DENSE_PMR::synchronized_pool_resource pool;

// segfaults if m_buckets is reused
{
map_default_resource = return_hello_world(&pool);
REQUIRE(map_default_resource.contains(0));
}
}

# endif
#endif

0 comments on commit a9893b3

Please sign in to comment.