Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new detail::atomic_ref_count utility for intrusive reference counting as a member (instead of inheritance), and migrates various ref-counted types in CAF core/net/test to use it. This aligns with issue #2307 by reducing duplicated ref-counting implementations and enabling ref counting for types that cannot use inheritance (e.g., chunk::data).
Changes:
- Add
caf::detail::atomic_ref_count(+memory_interface) and replaceatomic_ref_counted/plain_ref_countedusage across the codebase. - Update intrusive/COW utilities and many call sites/tests to use
strong_reference_count()instead ofunique()/get_reference_count(). - Add/adjust tests (including new
intrusive_cow_ptr.test.cpp) and update build/test configuration accordingly.
Reviewed changes
Copilot reviewed 78 out of 78 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| libcaf_test/caf/test/reporter.cpp | Switch test logger to member-based ref count |
| libcaf_net/caf/net/socket_manager.hpp | Replace inherited ref counting include/base |
| libcaf_net/caf/net/socket_manager.cpp | Use atomic_ref_count and route intrusive_ptr to disposable refs |
| libcaf_net/caf/net/multiplexer.hpp | Remove unused atomic ref-count include |
| libcaf_net/caf/net/multiplexer.cpp | Replace inherited ref counting; safer cleanup when pipe write fails |
| libcaf_net/caf/net/http/with.cpp | Replace inherited ref counting in producer impl |
| libcaf_net/caf/net/http/responder.hpp | Include ref_counted dependency |
| libcaf_core/caf/uri.hpp | Move URI impl to member-based ref count; API rename to strong_reference_count |
| libcaf_core/caf/uri.cpp | Remove old rc_ constructor |
| libcaf_core/caf/scheduler.test.cpp | Update tests to strong_reference_count() |
| libcaf_core/caf/ref_counted.hpp | Replace inherited ref counting with member atomic_ref_count |
| libcaf_core/caf/ref_counted.cpp | Make destructor noexcept |
| libcaf_core/caf/node_id.hpp | Update COW logic to strong_reference_count() |
| libcaf_core/caf/message_lifetime.test.cpp | Update reference count assertions |
| libcaf_core/caf/message_builder.test.cpp | Update reference count assertions; include make_counted |
| libcaf_core/caf/message_builder.cpp | Use strong_reference_count() for move-vs-copy decision |
| libcaf_core/caf/message.test.cpp | Update reference count assertions |
| libcaf_core/caf/message.hpp | Remove unique() helper; include adjustments |
| libcaf_core/caf/logger.cpp | Remove unused atomic ref-count include |
| libcaf_core/caf/intrusive_ptr.test.cpp | Update tests to strong_reference_count() |
| libcaf_core/caf/intrusive_ptr.hpp | Include build_config; minor cleanup |
| libcaf_core/caf/intrusive_cow_ptr.test.cpp | New test coverage for intrusive COW pointer |
| libcaf_core/caf/intrusive_cow_ptr.hpp | Rework unshare customization; remove reliance on internal intrusive_ptr stringification |
| libcaf_core/caf/fwd.hpp | Remove now-obsolete fwd declarations for old COW support |
| libcaf_core/caf/flow/subscription.hpp | Replace plain ref counting base with member atomic_ref_count |
| libcaf_core/caf/flow/subscription.cpp | Implement ref/deref via atomic_ref_count |
| libcaf_core/caf/flow/op/ucast.hpp | Replace plain ref counting with member atomic_ref_count |
| libcaf_core/caf/flow/op/retry.hpp | Delegate coordinated ref counting to base impl |
| libcaf_core/caf/flow/op/publish.hpp | Use atomic_ref_count via base member |
| libcaf_core/caf/flow/op/prefix_and_tail.hpp | Delegate coordinated ref counting to base impl |
| libcaf_core/caf/flow/op/on_error_resume_next.hpp | Delegate coordinated ref counting to base impl |
| libcaf_core/caf/flow/op/on_backpressure_buffer.hpp | Delegate coordinated ref counting to base impl |
| libcaf_core/caf/flow/op/merge.hpp | Delegate coordinated ref counting; simplify ref forwarding |
| libcaf_core/caf/flow/op/interval.cpp | Remove plain ref-count include |
| libcaf_core/caf/flow/op/hot.hpp | Replace inherited ref counting with member atomic_ref_count |
| libcaf_core/caf/flow/op/from_steps.hpp | Delegate coordinated ref counting to base impl |
| libcaf_core/caf/flow/op/from_resource.hpp | Replace inherited ref counting with member atomic_ref_count |
| libcaf_core/caf/flow/op/debounce.hpp | Delegate coordinated ref counting to base impl |
| libcaf_core/caf/flow/op/concat.hpp | Delegate coordinated ref counting to base impl |
| libcaf_core/caf/flow/op/cold.hpp | Replace inherited ref counting with member atomic_ref_count |
| libcaf_core/caf/flow/op/cache.hpp | Use atomic_ref_count via base member |
| libcaf_core/caf/flow/op/auto_connect.hpp | Replace inherited ref counting with member atomic_ref_count |
| libcaf_core/caf/flow/observer.hpp | Replace plain/inherited ref counting with member atomic_ref_count |
| libcaf_core/caf/flow/generation.test.cpp | Update reference count assertions |
| libcaf_core/caf/expected.test.cpp | Update reference count assertions; include make_counted |
| libcaf_core/caf/detail/plain_ref_counted.hpp | Remove legacy plain ref counting implementation |
| libcaf_core/caf/detail/plain_ref_counted.cpp | Remove legacy plain ref counting implementation |
| libcaf_core/caf/detail/monitor_action.hpp | Switch monitor_action to member ref count |
| libcaf_core/caf/detail/monitor_action.cpp | Drop old ref forwarding; keep dtor |
| libcaf_core/caf/detail/message_data.hpp | Rename get_reference_count to strong_reference_count; remove unique() |
| libcaf_core/caf/detail/memory_interface.hpp | New: declare allocation/deallocation interface marker |
| libcaf_core/caf/detail/behavior_impl.hpp | Update unique-check logic using strong refcount |
| libcaf_core/caf/detail/beacon.hpp | Switch beacon to member ref count |
| libcaf_core/caf/detail/beacon.cpp | Implement ref/deref via atomic_ref_count |
| libcaf_core/caf/detail/atomic_ref_counted.hpp | Remove legacy atomic ref counting base |
| libcaf_core/caf/detail/atomic_ref_counted.cpp | Remove legacy atomic ref counting base |
| libcaf_core/caf/detail/atomic_ref_count.hpp | New: member-based atomic reference count implementation |
| libcaf_core/caf/detail/asynchronous_logger.cpp | Switch default logger to member ref count |
| libcaf_core/caf/cow_vector.hpp | Remove unique() helper |
| libcaf_core/caf/cow_tuple.test.cpp | Update tests to strong_reference_count() |
| libcaf_core/caf/cow_tuple.hpp | Remove unique() helper |
| libcaf_core/caf/cow_string.test.cpp | Update tests to strong_reference_count() |
| libcaf_core/caf/cow_string.hpp | Rename get_reference_count to strong_reference_count; remove unique() |
| libcaf_core/caf/chunk.hpp | Switch chunk::data to atomic_ref_count + malloc/free interface marker |
| libcaf_core/caf/chunk.cpp | Remove custom deref implementation (now via atomic_ref_count) |
| libcaf_core/caf/behavior.test.cpp | Update reference count assertions |
| libcaf_core/caf/action.test.cpp | Update tests to strong_reference_count() |
| libcaf_core/caf/action.hpp | Switch default_action_impl to member ref count |
| libcaf_core/CMakeLists.txt | Remove old ref-count sources; add new test |
| .cppcheck-suppressions | Add suppressions for intentionally non-explicit constructors |
| libcaf_core/caf/async/producer_adapter.hpp | Switch producer impl to member ref count |
| libcaf_core/caf/async/mock_producer.test.hpp | Switch mock producer to member ref count |
| libcaf_core/caf/async/mock_producer.test.cpp | Implement ref/deref via atomic_ref_count |
| libcaf_core/caf/async/mock_consumer.test.hpp | Switch mock consumer to member ref count |
| libcaf_core/caf/async/mock_consumer.test.cpp | Implement ref/deref via atomic_ref_count |
| libcaf_core/caf/async/consumer_adapter.hpp | Switch consumer impl to member ref count |
| libcaf_core/caf/async/blocking_producer.hpp | Switch blocking producer impl to member ref count |
| libcaf_core/caf/async/batch.hpp | Switch batch::data to atomic_ref_count + malloc/free interface marker |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2308 +/- ##
==========================================
+ Coverage 72.86% 72.91% +0.04%
==========================================
Files 638 635 -3
Lines 30870 30866 -4
Branches 3324 3319 -5
==========================================
+ Hits 22493 22505 +12
+ Misses 6487 6473 -14
+ Partials 1890 1888 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The inheritance-based approach with `atomic_ref_counted` and `plain_ref_counted` did not work in all cases. For example, `chunk::data` cannot use inheritance and thus had to roll its own reference counting implementation. The new class `atomic_ref_count` is designed to be used as a member variable and works in all cases where we need an intrusive reference count. This approach simplifies the design and makes sure that we only have a single implementation.
Neverlord
force-pushed
the
issue/2307-intrusive-ref-count
branch
from
85f9344 to
c9e9e16
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The inheritance-based approach with
atomic_ref_countedandplain_ref_counteddid not work in all cases. For example,chunk::datacannot use inheritance and thus had to roll its own reference counting implementation.The new class
atomic_ref_countis designed to be used as a member variable and works in all cases where we need an intrusive reference count. This approach simplifies the design and makes sure that we only have a single implementation.This is the first part for #2307.