diff --git a/benchmarks/bench_reference.cpp b/benchmarks/bench_reference.cpp index b2fe9fe..d9e5768 100644 --- a/benchmarks/bench_reference.cpp +++ b/benchmarks/bench_reference.cpp @@ -27,6 +27,18 @@ template struct PointerTraits> { return std::make_shared(std::forward(args)...); } + static pointer_type copy(const pointer_type &ptr) { + return ptr; // std::shared_ptr copies implicitly + } + + static weak_type as_weak(const pointer_type &ptr) { + return ptr; // std::weak_ptr converts implicitly from std::shared_ptr + } + + static weak_type copy_weak(const weak_type &weak) { + return weak; // std::weak_ptr copies implicitly + } + static const char *name() { return "std::shared_ptr"; } static const char *weak_name() { return "std::weak_ptr"; } }; @@ -39,6 +51,18 @@ template struct PointerTraits> { return make_ref(std::forward(args)...); } + static pointer_type copy(const pointer_type &ptr) { + return ptr.copy(); // Ref requires explicit copy + } + + static weak_type as_weak(const pointer_type &ptr) { + return ptr.as_weak(); // Ref requires explicit as_weak + } + + static weak_type copy_weak(const weak_type &weak) { + return weak.copy(); // WeakRef requires explicit copy + } + static const char *name() { return "Ref"; } static const char *weak_name() { return "WeakRef"; } }; @@ -67,7 +91,7 @@ void benchmark_copy(ankerl::nanobench::Bench &bench) { auto original = Traits::make(TestObject{123}); bench.run(std::string(Traits::name()) + " copy", [&] { - auto copy = original; + auto copy = Traits::copy(original); ankerl::nanobench::doNotOptimizeAway(copy); }); } @@ -91,9 +115,9 @@ void benchmark_weak_copy(ankerl::nanobench::Bench &bench) { force_multithreaded(); auto strong_ptr = Traits::make(TestObject{123}); - typename Traits::weak_type weak_original = strong_ptr; + typename Traits::weak_type weak_original = Traits::as_weak(strong_ptr); bench.run(std::string(Traits::weak_name()) + " copy", [&] { - auto weak_copy = weak_original; + auto weak_copy = Traits::copy_weak(weak_original); ankerl::nanobench::doNotOptimizeAway(weak_copy); }); } @@ -103,7 +127,7 @@ void benchmark_weak_move(ankerl::nanobench::Bench &bench) { using Traits = PointerTraits; auto strong_ptr = Traits::make(TestObject{123}); - typename Traits::weak_type weak_original = strong_ptr; + typename Traits::weak_type weak_original = Traits::as_weak(strong_ptr); bench.run(std::string(Traits::weak_name()) + " move", [&] { auto weak_moved = std::move(weak_original); ankerl::nanobench::doNotOptimizeAway(weak_moved); @@ -126,7 +150,7 @@ void benchmark_weak_lock_success(ankerl::nanobench::Bench &bench) { using Traits = PointerTraits; auto strong_ptr = Traits::make(TestObject{789}); - typename Traits::weak_type weak_ptr = strong_ptr; + typename Traits::weak_type weak_ptr = Traits::as_weak(strong_ptr); bench.run(std::string(Traits::weak_name()) + " lock success", [&] { auto locked = weak_ptr.lock(); ankerl::nanobench::doNotOptimizeAway(locked); @@ -140,7 +164,7 @@ void benchmark_weak_lock_failure(ankerl::nanobench::Bench &bench) { typename Traits::weak_type weak_ptr; { auto strong_ptr = Traits::make(TestObject{999}); - weak_ptr = strong_ptr; + weak_ptr = Traits::as_weak(strong_ptr); } bench.run(std::string(Traits::weak_name()) + " lock failure", [&] { auto locked = weak_ptr.lock(); @@ -163,7 +187,7 @@ void benchmark_multithreaded_copy(ankerl::nanobench::Bench &bench, for (int i = 0; i < num_threads - 1; ++i) { background_threads.emplace_back([&]() { while (keep_running.load(std::memory_order_relaxed)) { - auto copy = ptr; + auto copy = Traits::copy(ptr); ankerl::nanobench::doNotOptimizeAway(copy); } }); @@ -171,7 +195,7 @@ void benchmark_multithreaded_copy(ankerl::nanobench::Bench &bench, // Benchmark the foreground thread under contention bench.run(std::string(Traits::name()) + " copy under contention", [&] { - auto copy = ptr; + auto copy = Traits::copy(ptr); ankerl::nanobench::doNotOptimizeAway(copy); }); @@ -189,7 +213,7 @@ void benchmark_multithreaded_weak_lock(ankerl::nanobench::Bench &bench, // Create the shared object and weak reference outside the benchmark auto strong_ptr = Traits::make(TestObject{789}); - typename Traits::weak_type weak_ptr = strong_ptr; + typename Traits::weak_type weak_ptr = Traits::as_weak(strong_ptr); // Create background threads that will create contention std::atomic keep_running{true}; @@ -224,7 +248,7 @@ void benchmark_weak_copy_with_strong_contention(ankerl::nanobench::Bench &bench, // Create the shared object and weak reference outside the benchmark auto strong_ptr = Traits::make(TestObject{456}); - typename Traits::weak_type weak_ptr = strong_ptr; + typename Traits::weak_type weak_ptr = Traits::as_weak(strong_ptr); // Create background threads copying the strong pointer std::atomic keep_running{true}; @@ -233,7 +257,7 @@ void benchmark_weak_copy_with_strong_contention(ankerl::nanobench::Bench &bench, for (int i = 0; i < num_threads - 1; ++i) { background_threads.emplace_back([&]() { while (keep_running.load(std::memory_order_relaxed)) { - auto copy = strong_ptr; + auto copy = Traits::copy(strong_ptr); ankerl::nanobench::doNotOptimizeAway(copy); } }); @@ -242,7 +266,7 @@ void benchmark_weak_copy_with_strong_contention(ankerl::nanobench::Bench &bench, // Benchmark weak reference copying under strong reference contention bench.run(std::string(Traits::weak_name()) + " copy with strong contention", [&] { - auto weak_copy = weak_ptr; + auto weak_copy = Traits::copy_weak(weak_ptr); ankerl::nanobench::doNotOptimizeAway(weak_copy); }); @@ -260,7 +284,7 @@ void benchmark_strong_copy_with_weak_contention(ankerl::nanobench::Bench &bench, // Create the shared object and weak reference outside the benchmark auto strong_ptr = Traits::make(TestObject{789}); - typename Traits::weak_type weak_ptr = strong_ptr; + typename Traits::weak_type weak_ptr = Traits::as_weak(strong_ptr); // Create background threads copying the weak pointer std::atomic keep_running{true}; @@ -269,7 +293,7 @@ void benchmark_strong_copy_with_weak_contention(ankerl::nanobench::Bench &bench, for (int i = 0; i < num_threads - 1; ++i) { background_threads.emplace_back([&]() { while (keep_running.load(std::memory_order_relaxed)) { - auto weak_copy = weak_ptr; + auto weak_copy = Traits::copy_weak(weak_ptr); ankerl::nanobench::doNotOptimizeAway(weak_copy); } }); @@ -277,7 +301,7 @@ void benchmark_strong_copy_with_weak_contention(ankerl::nanobench::Bench &bench, // Benchmark strong reference copying under weak reference contention bench.run(std::string(Traits::name()) + " copy with weak contention", [&] { - auto strong_copy = strong_ptr; + auto strong_copy = Traits::copy(strong_ptr); ankerl::nanobench::doNotOptimizeAway(strong_copy); }); diff --git a/src/reference.hpp b/src/reference.hpp index cc69773..714c6a3 100644 --- a/src/reference.hpp +++ b/src/reference.hpp @@ -22,8 +22,8 @@ * Basic usage: * @code * auto obj = make_ref(args...); // Create managed object - * auto copy = obj; // Copy (thread-safe) - * WeakRef weak = obj; // Create weak reference + * auto copy = obj.copy(); // Explicit copy (thread-safe) + * WeakRef weak = obj.as_weak(); // Create weak reference * auto locked = weak.lock(); // Try to promote to strong * @endcode * @@ -31,6 +31,9 @@ * safely copy, move, and destroy references to the same object. */ +// Forward declaration +template struct WeakRef; + namespace detail { struct ControlBlock { std::atomic strong_count; @@ -126,57 +129,28 @@ template struct Ref { ~Ref() { release(); } /** - * @brief Copy constructor - increments strong reference count + * @brief Copy constructor - deleted to prevent accidental copies + * Use copy() method for explicit copying */ - Ref(const Ref &other) noexcept - : ptr(other.ptr), control_block(other.control_block) { - if (control_block) { - control_block->increment_strong(); - } - } + Ref(const Ref &other) = delete; /** - * @brief Converting copy constructor for polymorphism (Derived -> Base) + * @brief Converting copy constructor - deleted to prevent accidental copies + * Use copy() method for explicit copying */ - template - Ref(const Ref &other) noexcept - requires std::is_convertible_v - : ptr(other.ptr), control_block(other.control_block) { - if (control_block) { - control_block->increment_strong(); - } - } + template Ref(const Ref &other) = delete; /** - * @brief Copy assignment operator + * @brief Copy assignment operator - deleted to prevent accidental copies + * Use copy() method for explicit copying */ - Ref &operator=(const Ref &other) noexcept { - if (this != &other) { - release(); - ptr = other.ptr; - control_block = other.control_block; - if (control_block) { - control_block->increment_strong(); - } - } - return *this; - } + Ref &operator=(const Ref &other) = delete; /** - * @brief Converting assignment operator for polymorphism (Derived -> Base) + * @brief Converting assignment operator - deleted to prevent accidental + * copies Use copy() method for explicit copying */ - template - Ref &operator=(const Ref &other) noexcept - requires std::is_convertible_v - { - release(); - ptr = other.ptr; - control_block = other.control_block; - if (control_block) { - control_block->increment_strong(); - } - return *this; - } + template Ref &operator=(const Ref &other) = delete; /** * @brief Move constructor - transfers ownership @@ -228,6 +202,28 @@ template struct Ref { return *this; } + /** + * @brief Explicitly create a copy with shared ownership + * @return New Ref that shares ownership of the same object + */ + [[nodiscard]] Ref copy() const noexcept { + if (control_block) { + control_block->increment_strong(); + } + return Ref(ptr, control_block); + } + + /** + * @brief Create a WeakRef that observes this object + * @return New WeakRef that observes the same object + */ + [[nodiscard]] WeakRef as_weak() const noexcept { + if (control_block) { + control_block->increment_weak(); + } + return WeakRef(control_block); + } + /** * @brief Reset to empty state */ @@ -347,76 +343,40 @@ template struct WeakRef { ~WeakRef() { release(); } /** - * @brief Copy constructor from WeakRef + * @brief Copy constructor from WeakRef - deleted to prevent accidental copies + * Use copy() method for explicit copying */ - WeakRef(const WeakRef &other) noexcept : control_block(other.control_block) { - if (control_block) { - control_block->increment_weak(); - } - } + WeakRef(const WeakRef &other) = delete; /** - * @brief Copy constructor from Ref + * @brief Copy constructor from Ref - deleted to prevent accidental copies + * Use copy() method for explicit copying */ - WeakRef(const Ref &ref) noexcept : control_block(ref.control_block) { - if (control_block) { - control_block->increment_weak(); - } - } + WeakRef(const Ref &ref) = delete; /** - * @brief Converting copy constructor from WeakRef for polymorphism + * @brief Converting copy constructor from WeakRef - deleted to prevent + * accidental copies Use copy() method for explicit copying */ - template - WeakRef(const WeakRef &other) noexcept - requires std::is_convertible_v - : control_block(other.control_block) { - if (control_block) { - control_block->increment_weak(); - } - } + template WeakRef(const WeakRef &other) = delete; /** - * @brief Converting copy constructor from Ref for polymorphism + * @brief Converting copy constructor from Ref - deleted to prevent accidental + * copies Use copy() method for explicit copying */ - template - WeakRef(const Ref &ref) noexcept - requires std::is_convertible_v - : control_block(ref.control_block) { - if (control_block) { - control_block->increment_weak(); - } - } + template WeakRef(const Ref &ref) = delete; /** - * @brief Converting copy assignment from WeakRef for polymorphism + * @brief Converting copy assignment from WeakRef - deleted to prevent + * accidental copies Use copy() method for explicit copying */ - template - WeakRef &operator=(const WeakRef &other) noexcept - requires std::is_convertible_v - { - release(); - control_block = other.control_block; - if (control_block) { - control_block->increment_weak(); - } - return *this; - } + template WeakRef &operator=(const WeakRef &other) = delete; /** - * @brief Converting copy assignment from Ref for polymorphism + * @brief Converting copy assignment from Ref - deleted to prevent accidental + * copies Use copy() method for explicit copying */ - template - WeakRef &operator=(const Ref &ref) noexcept - requires std::is_convertible_v - { - release(); - control_block = ref.control_block; - if (control_block) { - control_block->increment_weak(); - } - return *this; - } + template WeakRef &operator=(const Ref &ref) = delete; /** * @brief Converting move constructor from WeakRef for polymorphism @@ -442,30 +402,16 @@ template struct WeakRef { } /** - * @brief Copy assignment from WeakRef + * @brief Copy assignment from WeakRef - deleted to prevent accidental copies + * Use copy() method for explicit copying */ - WeakRef &operator=(const WeakRef &other) noexcept { - if (this != &other) { - release(); - control_block = other.control_block; - if (control_block) { - control_block->increment_weak(); - } - } - return *this; - } + WeakRef &operator=(const WeakRef &other) = delete; /** - * @brief Copy assignment from Ref + * @brief Copy assignment from Ref - deleted to prevent accidental copies + * Use copy() method for explicit copying */ - WeakRef &operator=(const Ref &ref) noexcept { - release(); - control_block = ref.control_block; - if (control_block) { - control_block->increment_weak(); - } - return *this; - } + WeakRef &operator=(const Ref &ref) = delete; /** * @brief Move constructor @@ -486,6 +432,17 @@ template struct WeakRef { return *this; } + /** + * @brief Explicitly create a copy with shared weak reference + * @return New WeakRef that observes the same object + */ + [[nodiscard]] WeakRef copy() const noexcept { + if (control_block) { + control_block->increment_weak(); + } + return WeakRef(control_block); + } + /** * @brief Reset to empty state */ @@ -550,6 +507,8 @@ private: * @code * auto obj = make_ref(arg1, arg2); * auto empty_vec = make_ref>(); + * auto obj_copy = obj.copy(); // Explicit copy + * WeakRef weak = obj.as_weak(); // Create weak reference * @endcode * * Thread safety: Safe to call from multiple threads simultaneously. diff --git a/src/server.cpp b/src/server.cpp index 596cc24..fee574b 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -25,7 +25,7 @@ Ref Server::create(const weaseldb::Config &config, ConnectionHandler &handler, const std::vector &listen_fds) { auto result = make_ref(config, handler, listen_fds); - result->self_ = result; + result->self_ = result.as_weak(); return result; } @@ -218,7 +218,7 @@ int Server::create_local_connection() { // Create Connection object auto connection = std::unique_ptr(new Connection( addr, server_fd, connection_id_.fetch_add(1, std::memory_order_relaxed), - epoll_index, &handler_, self_)); + epoll_index, &handler_, self_.copy())); // Store in registry connection_registry_.store(server_fd, std::move(connection)); @@ -422,7 +422,7 @@ void Server::start_io_threads(std::vector &threads) { batch[batch_count] = std::unique_ptr(new Connection( addr, fd, connection_id_.fetch_add(1, std::memory_order_relaxed), - epoll_index, &handler_, self_)); + epoll_index, &handler_, self_.copy())); batch_events[batch_count] = EPOLLIN; // New connections always start with read batch_count++; diff --git a/tests/test_reference.cpp b/tests/test_reference.cpp index a86ea1b..e4d41ae 100644 --- a/tests/test_reference.cpp +++ b/tests/test_reference.cpp @@ -52,9 +52,9 @@ TEST_CASE("Ref basic functionality") { CHECK((*ref).value == 42); } - SUBCASE("copy construction increments reference count") { + SUBCASE("explicit copy increments reference count") { auto ref1 = make_ref(123); - auto ref2 = ref1; + auto ref2 = ref1.copy(); CHECK(ref1); CHECK(ref2); @@ -63,11 +63,11 @@ TEST_CASE("Ref basic functionality") { CHECK(ref2->value == 123); } - SUBCASE("copy assignment works correctly") { + SUBCASE("explicit copy assignment works correctly") { auto ref1 = make_ref(100); auto ref2 = make_ref(200); - ref2 = ref1; + ref2 = ref1.copy(); CHECK(ref1.get() == ref2.get()); CHECK(ref1->value == 100); CHECK(ref2->value == 100); @@ -109,7 +109,7 @@ TEST_CASE("Ref basic functionality") { TEST_CASE("WeakRef basic functionality") { SUBCASE("construction from Ref") { auto ref = make_ref(333); - WeakRef weak_ref = ref; + WeakRef weak_ref = ref.as_weak(); auto locked = weak_ref.lock(); CHECK(locked); @@ -121,7 +121,7 @@ TEST_CASE("WeakRef basic functionality") { WeakRef weak_ref; { auto ref = make_ref(444); - weak_ref = ref; + weak_ref = ref.as_weak(); } // ref goes out of scope, object should be destroyed @@ -131,8 +131,8 @@ TEST_CASE("WeakRef basic functionality") { SUBCASE("copy and move semantics") { auto ref = make_ref(666); - WeakRef weak1 = ref; - WeakRef weak2 = weak1; // copy + WeakRef weak1 = ref.as_weak(); + WeakRef weak2 = weak1.copy(); // explicit copy WeakRef weak3 = std::move(weak1); // move auto locked2 = weak2.lock(); @@ -160,7 +160,7 @@ TEST_CASE("Ref thread safety") { start_latch.arrive_and_wait(); for (int j = 0; j < copies_per_thread; ++j) { - auto copy = ref; + auto copy = ref.copy(); CHECK(copy); CHECK(copy->value == 777); } @@ -191,7 +191,7 @@ TEST_CASE("Control block cleanup race condition test") { WeakRef ptr2; auto setup = [&]() { ptr1 = make_ref(0); - ptr2 = ptr1; + ptr2 = ptr1.as_weak(); }; // Barrier for synchronization - 2 participants (main thread + worker thread) @@ -243,7 +243,7 @@ TEST_CASE("WeakRef prevents circular references") { // Create object and weak reference { auto ref = make_ref(123); - weak_ref = ref; + weak_ref = ref.as_weak(); // Should be able to lock while object exists auto locked = weak_ref.lock(); @@ -262,8 +262,8 @@ TEST_CASE("WeakRef prevents circular references") { auto child = make_ref(2); // Create potential cycle - parent->next = child; // Strong reference: parent → child - child->parent = parent; // WeakRef: child ⇝ parent (breaks cycle) + parent->next = child.copy(); // Strong reference: parent → child + child->parent = parent.as_weak(); // WeakRef: child ⇝ parent (breaks cycle) CHECK(parent->data == 1); CHECK(child->data == 2); @@ -286,7 +286,7 @@ TEST_CASE("Polymorphic Ref conversions") { CHECK(derived_ref->get_value() == 30); // 10 + 20 // Convert Ref to Ref - Ref base_ref = derived_ref; + Ref base_ref = derived_ref.copy(); CHECK(base_ref); CHECK(base_ref->get_value() == 30); // Virtual dispatch works CHECK(base_ref->base_value == 10); @@ -303,7 +303,7 @@ TEST_CASE("Polymorphic Ref conversions") { CHECK(base_ref->get_value() == 100); // Assign derived to base - base_ref = derived_ref; + base_ref = derived_ref.copy(); CHECK(base_ref->get_value() == 20); // 5 + 15 CHECK(base_ref.get() == derived_ref.get()); } @@ -338,7 +338,7 @@ TEST_CASE("Polymorphic Ref conversions") { CHECK(another_derived->get_value() == 24); // 6 * 4 // Convert to base - Ref base_ref = another_derived; + Ref base_ref = another_derived.copy(); CHECK(base_ref->get_value() == 24); // Virtual dispatch CHECK(base_ref.get() == another_derived.get()); } @@ -349,10 +349,10 @@ TEST_CASE("Polymorphic WeakRef conversions") { auto derived_ref = make_ref(3, 7); // Create WeakRef - WeakRef weak_derived = derived_ref; + WeakRef weak_derived = derived_ref.as_weak(); // Convert to WeakRef - WeakRef weak_base = weak_derived; + WeakRef weak_base = weak_derived.copy(); // Both should lock to same object auto locked_derived = weak_derived.lock(); @@ -368,11 +368,11 @@ TEST_CASE("Polymorphic WeakRef conversions") { auto derived_ref = make_ref(4, 6); auto base_ref = make_ref(999); - WeakRef weak_derived = derived_ref; - WeakRef weak_base = base_ref; + WeakRef weak_derived = derived_ref.as_weak(); + WeakRef weak_base = base_ref.as_weak(); // Assign derived weak ref to base weak ref - weak_base = weak_derived; + weak_base = weak_derived.copy(); auto locked = weak_base.lock(); CHECK(locked); @@ -384,7 +384,7 @@ TEST_CASE("Polymorphic WeakRef conversions") { auto derived_ref = make_ref(2, 8); // Create WeakRef directly from Ref - WeakRef weak_base = derived_ref; + WeakRef weak_base = derived_ref.as_weak(); auto locked = weak_base.lock(); CHECK(locked); @@ -394,7 +394,7 @@ TEST_CASE("Polymorphic WeakRef conversions") { SUBCASE("WeakRef move operations") { auto derived_ref = make_ref(1, 9); - WeakRef weak_derived = derived_ref; + WeakRef weak_derived = derived_ref.as_weak(); // Move construct WeakRef weak_base = std::move(weak_derived); @@ -414,7 +414,7 @@ TEST_CASE("Polymorphic edge cases") { CHECK(!empty_derived); // Convert empty derived to base - Ref empty_base = empty_derived; + Ref empty_base = empty_derived.copy(); CHECK(!empty_base); // Move empty derived to base @@ -427,7 +427,7 @@ TEST_CASE("Polymorphic edge cases") { CHECK(!empty_weak_derived.lock()); // Convert empty weak derived to weak base - WeakRef empty_weak_base = empty_weak_derived; + WeakRef empty_weak_base = empty_weak_derived.copy(); CHECK(!empty_weak_base.lock()); } @@ -435,7 +435,7 @@ TEST_CASE("Polymorphic edge cases") { auto derived_ref = make_ref(5, 5); // Ref → WeakRef - WeakRef weak_base_from_ref = derived_ref; + WeakRef weak_base_from_ref = derived_ref.as_weak(); // WeakRef → Ref via lock auto base_ref_from_weak = weak_base_from_ref.lock(); @@ -457,5 +457,5 @@ TEST_CASE("Self-referencing WeakRef pattern") { WeakRef self_; }; auto x = make_ref(); - x->self_ = x; + x->self_ = x.as_weak(); }