Skip to content

Commit 9fccef5

Browse files
NeverlordNeverlord
authored
Merge pull request #2409
Redesign `actor_addr` as value type
2 parents 42882ae + 44d0bf1 commit 9fccef5

46 files changed

Lines changed: 475 additions & 557 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎CHANGELOG.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,14 @@ is based on [Keep a Changelog](https://keepachangelog.com).
6767
- The `caf::async::producer::on_consumer_demand` callback now takes a second
6868
`bool unblocked` argument. Please refer to the documentation of the `producer`
6969
class for more details.
70+
- The class `actor_addr` has been redesigned from scratch. Prior to CAF 2.0, an
71+
`actor_addr` held a weak pointer to an actor. This made it fundamentally
72+
unsafe to send across the network. Since the only use case for `actor_addr` in
73+
CAF is to identify an actor from an `exit_msg` or `down_msg`, we have
74+
re-implemented it as a simple pair of `actor_id` and `node_id`. With this
75+
change, `actor_addr` is now safe to send across the network. This change
76+
should be (mostly) transparent to users with the exception of `actor_cast`,
77+
which no longer can convert `actor_addr` to other actor handles.
7078

7179
### Deprecated
7280

@@ -226,6 +234,11 @@ is based on [Keep a Changelog](https://keepachangelog.com).
226234
points for the same functionality only adds redundancy.
227235
- Removed the `node_id::can_parse` method and the `parse` function for node IDs.
228236
Both are leftovers from older versions of CAF and no longer serve any purpose.
237+
- Related to the `actor_addr` change, we have removed the type ID for
238+
`weak_actor_ptr` and inspectors no longer support `weak_actor_ptr` values.
239+
This means that CAF messages may no longer contain a `weak_actor_ptr`. While
240+
this is technically a breaking API change, we consider it a bug fix since
241+
sending a `weak_actor_ptr` silently failed in earlier CAF versions.
229242

230243
## [1.1.0] - 2025-07-25
231244

‎libcaf_core/CMakeLists.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,14 +169,15 @@ caf_add_component(
169169
caf/detail/bounds_checker.test.cpp
170170
caf/detail/cleanup_and_release.cpp
171171
caf/detail/cleanup_and_release.test.cpp
172+
caf/detail/compare.cpp
172173
caf/detail/config_consumer.cpp
173174
caf/detail/config_consumer.test.cpp
174175
caf/detail/counted_disposable.cpp
175176
caf/detail/counted_disposable.test.cpp
176177
caf/detail/critical.cpp
177178
caf/detail/current_actor.cpp
178-
caf/detail/default_mailbox.cpp
179179
caf/detail/default_actor_handle_codec.cpp
180+
caf/detail/default_mailbox.cpp
180181
caf/detail/default_mailbox.test.cpp
181182
caf/detail/default_thread_count.cpp
182183
caf/detail/format.test.cpp

‎libcaf_core/caf/abstract_actor.cpp‎

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -117,25 +117,33 @@ void abstract_actor::del_monitor(abstract_actor* observed,
117117

118118
// -- linking ------------------------------------------------------------------
119119

120-
void abstract_actor::link_to(const actor_addr& other) {
121-
auto lg = log::core::trace("other = {}", other);
122-
link_to(actor_cast<strong_actor_ptr>(other));
123-
}
124-
125120
void abstract_actor::unlink_from(const actor_addr& other) {
126121
auto lg = log::core::trace("other = {}", other);
127-
if (!other)
122+
if (other.id() == invalid_actor_id)
128123
return;
129-
if (auto hdl = actor_cast<strong_actor_ptr>(other)) {
130-
unlink_from(hdl);
124+
// Try to resolve `other` to a weak pointer.
125+
weak_actor_ptr other_ptr;
126+
exclusive_critical_section([this, &other, &other_ptr] { //
127+
internal::attachable_predicate::extractor extractor{&other, &other_ptr};
128+
auto pred = internal::attachable_predicate::linked_to(&extractor);
129+
std::ignore = this->attachables_.any_of(pred);
130+
});
131+
// If there's no link attachable for `other`, we're done.
132+
if (!other_ptr) {
133+
return;
134+
}
135+
// If `other` is still alive, unlink by calling remove_link.
136+
if (auto hdl = other_ptr.lock()) {
137+
CAF_ASSERT(hdl.get() != ctrl());
138+
remove_link(hdl->get());
131139
return;
132140
}
133141
// Promoting weak to strong reference fails if-and-only-if the strong
134142
// reference count is already at 0 which means the other actor must have
135-
// terminated. However, we call this overload automatically after receiving an
136-
// exit message to clean up leftover state.
137-
auto pred = internal::attachable_predicate::linked_to(other.ptr().ctrl());
138-
exclusive_critical_section([this, &pred] { //
143+
// terminated already. Clean up leftover state.
144+
exclusive_critical_section([this, &other] { //
145+
internal::attachable_predicate::extractor extractor{&other, nullptr};
146+
auto pred = internal::attachable_predicate::linked_to(&extractor);
139147
attachables_.erase_first_if(pred);
140148
});
141149
}
@@ -160,7 +168,7 @@ actor_system& abstract_actor::home_system() const noexcept {
160168
}
161169

162170
actor_addr abstract_actor::address() const noexcept {
163-
return actor_addr{ctrl_, add_ref};
171+
return {id(), node()};
164172
}
165173

166174
abstract_actor* abstract_actor::current() noexcept {
@@ -239,7 +247,8 @@ void abstract_actor::add_link(abstract_actor* x) {
239247
CAF_ASSERT(x != nullptr);
240248
error fail_state;
241249
bool send_exit_immediately = false;
242-
auto tmp = internal::attachable_factory::make_link(x->address());
250+
auto tmp = internal::attachable_factory::make_link(
251+
weak_actor_ptr{x->ctrl(), add_ref});
243252
joined_exclusive_critical_section(this, x, [&] {
244253
if (getf(is_terminated_flag)) {
245254
fail_state = fail_state_;
@@ -276,7 +285,8 @@ bool abstract_actor::add_backlink(abstract_actor* x) {
276285
}
277286
auto pred = internal::attachable_predicate::linked_to(x->ctrl());
278287
if (!attachables_.any_of(pred)) {
279-
auto tmp = internal::attachable_factory::make_link(x->address());
288+
auto tmp = internal::attachable_factory::make_link(
289+
weak_actor_ptr{x->ctrl(), add_ref});
280290
attachables_.push(std::move(tmp));
281291
return true;
282292
}
@@ -293,7 +303,7 @@ bool abstract_actor::remove_backlink(abstract_actor* x) {
293303
void abstract_actor::clear_incoming_edges(const actor_addr& other) {
294304
auto lg = log::core::trace("other = {}", other);
295305
exclusive_critical_section([this, &other] { //
296-
auto iter = incoming_edges_.find(other.ptr().ctrl());
306+
auto iter = incoming_edges_.find(other);
297307
if (iter != incoming_edges_.end()) {
298308
incoming_edges_.erase(iter);
299309
}

‎libcaf_core/caf/abstract_actor.hpp‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,6 @@ class CAF_CORE_EXPORT abstract_actor : public abstract_ref_counted {
9696

9797
// -- linking ----------------------------------------------------------------
9898

99-
/// Links this actor to `other`.
100-
void link_to(const actor_addr& other);
101-
10299
/// Links this actor to `other`.
103100
template <class ActorHandle>
104101
void link_to(const ActorHandle& other) {
@@ -108,7 +105,7 @@ class CAF_CORE_EXPORT abstract_actor : public abstract_ref_counted {
108105
}
109106
}
110107

111-
/// Unlinks this actor from `addr`.
108+
/// Unlinks this actor from `other`.
112109
void unlink_from(const actor_addr& other);
113110

114111
/// Links this actor to `hdl`.

‎libcaf_core/caf/actor.cpp‎

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,7 @@
55
#include "caf/actor.hpp"
66

77
#include "caf/actor_addr.hpp"
8-
#include "caf/actor_proxy.hpp"
9-
#include "caf/deserializer.hpp"
10-
#include "caf/event_based_actor.hpp"
11-
#include "caf/local_actor.hpp"
12-
#include "caf/make_actor.hpp"
138
#include "caf/scoped_actor.hpp"
14-
#include "caf/serializer.hpp"
15-
16-
#include <cassert>
17-
#include <utility>
189

1910
namespace caf {
2011

@@ -55,24 +46,15 @@ actor& actor::operator=(const scoped_actor& x) {
5546
return *this;
5647
}
5748

58-
intptr_t actor::compare(const actor& x) const noexcept {
59-
return actor_addr::compare(ptr_.get(), x.ptr_.get());
60-
}
61-
62-
intptr_t actor::compare(const actor_addr& x) const noexcept {
63-
return actor_addr::compare(ptr_.get(), actor_cast<actor_control_block*>(x));
64-
}
65-
66-
intptr_t actor::compare(const strong_actor_ptr& x) const noexcept {
67-
return actor_addr::compare(ptr_.get(), x.get());
68-
}
69-
7049
void actor::swap(actor& other) noexcept {
7150
ptr_.swap(other.ptr_);
7251
}
7352

7453
actor_addr actor::address() const noexcept {
75-
return actor_cast<actor_addr>(ptr_);
54+
if (ptr_) {
55+
return {ptr_->id(), ptr_->node()};
56+
}
57+
return {};
7658
}
7759

7860
bool operator==(const actor& lhs, abstract_actor* rhs) {

‎libcaf_core/caf/actor.hpp‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,16 @@
1010
#include "caf/add_ref.hpp"
1111
#include "caf/adopt_ref.hpp"
1212
#include "caf/caf_deprecated.hpp"
13-
#include "caf/config.hpp"
1413
#include "caf/detail/assert.hpp"
1514
#include "caf/detail/comparable.hpp"
16-
#include "caf/detail/concepts.hpp"
15+
#include "caf/detail/compare.hpp"
1716
#include "caf/detail/core_export.hpp"
1817
#include "caf/fwd.hpp"
19-
#include "caf/message.hpp"
18+
#include "caf/hash/fnv.hpp"
2019

2120
#include <cstddef>
2221
#include <cstdint>
2322
#include <string>
24-
#include <type_traits>
2523
#include <utility>
2624

2725
namespace caf {
@@ -120,11 +118,21 @@ class CAF_CORE_EXPORT actor : detail::comparable<actor>,
120118
return ptr_->managed();
121119
}
122120

123-
intptr_t compare(const actor&) const noexcept;
121+
intptr_t compare(const actor_control_block* other) const noexcept {
122+
return detail::compare(get(), other);
123+
}
124124

125-
intptr_t compare(const actor_addr&) const noexcept;
125+
intptr_t compare(const actor& other) const noexcept {
126+
return detail::compare(get(), other.get());
127+
}
126128

127-
intptr_t compare(const strong_actor_ptr&) const noexcept;
129+
intptr_t compare(const actor_addr& other) const noexcept {
130+
return detail::compare(get(), other);
131+
}
132+
133+
intptr_t compare(const strong_actor_ptr& other) const noexcept {
134+
return detail::compare(get(), other.get());
135+
}
128136

129137
CAF_DEPRECATED("construct using add_ref or adopt_ref instead")
130138
actor(actor_control_block*, bool);
@@ -183,12 +191,16 @@ CAF_CORE_EXPORT bool operator!=(abstract_actor* lhs, const actor& rhs);
183191

184192
} // namespace caf
185193

186-
// allow actor to be used in hash maps
187194
namespace std {
195+
188196
template <>
189197
struct hash<caf::actor> {
190198
size_t operator()(const caf::actor& ref) const noexcept {
191-
return static_cast<size_t>(ref ? ref->id() : 0);
199+
if (!ref) {
200+
return 0;
201+
}
202+
return caf::hash::fnv<size_t>::compute(ref.id(), ref.node());
192203
}
193204
};
205+
194206
} // namespace std

‎libcaf_core/caf/actor_addr.cpp‎

Lines changed: 43 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -4,78 +4,68 @@
44

55
#include "caf/actor_addr.hpp"
66

7-
#include "caf/actor.hpp"
8-
#include "caf/config.hpp"
9-
#include "caf/deserializer.hpp"
10-
#include "caf/local_actor.hpp"
7+
#include "caf/abstract_actor.hpp"
8+
#include "caf/actor_control_block.hpp"
9+
#include "caf/detail/compare.hpp"
10+
#include "caf/detail/print.hpp"
1111
#include "caf/node_id.hpp"
12-
#include "caf/proxy_registry.hpp"
13-
#include "caf/serializer.hpp"
1412

1513
namespace caf {
1614

17-
actor_addr::actor_addr(std::nullptr_t) {
18-
// nop
15+
actor_addr::actor_addr(actor_control_block* ptr) noexcept {
16+
if (ptr != nullptr) {
17+
id_ = ptr->id();
18+
node_ = ptr->node();
19+
}
1920
}
20-
21-
actor_addr& actor_addr::operator=(std::nullptr_t) {
22-
ptr_.reset();
23-
return *this;
24-
}
25-
26-
actor_addr::actor_addr(actor_control_block* ptr) : ptr_(ptr, add_ref) {
27-
// nop
28-
}
29-
30-
CAF_PUSH_DEPRECATED_WARNING
31-
actor_addr::actor_addr(actor_control_block* ptr, bool increase_ref_count)
32-
: ptr_(ptr, increase_ref_count) {
33-
// nop
21+
intptr_t actor_addr::compare(const actor_addr& other) const noexcept {
22+
return detail::compare(*this, other);
3423
}
35-
CAF_POP_WARNINGS
3624

37-
actor_addr::actor_addr(actor_control_block* ptr, add_ref_t)
38-
: ptr_(ptr, add_ref) {
39-
// nop
25+
intptr_t actor_addr::compare(const strong_actor_ptr& other) const noexcept {
26+
return detail::compare(*this, other.get());
4027
}
4128

42-
actor_addr::actor_addr(actor_control_block* ptr, adopt_ref_t)
43-
: ptr_(ptr, adopt_ref) {
44-
// nop
29+
intptr_t actor_addr::compare(const weak_actor_ptr& other) const noexcept {
30+
return detail::compare(*this, other.ctrl());
4531
}
4632

47-
intptr_t actor_addr::compare(const actor_control_block* lhs,
48-
const actor_control_block* rhs) {
49-
// invalid actors are always "less" than valid actors
50-
if (lhs == nullptr)
51-
return rhs != nullptr ? -1 : 0;
52-
if (rhs == nullptr)
53-
return 1;
54-
// check for identity
55-
if (lhs == rhs)
56-
return 0;
57-
// check for equality (a decorator is equal to the actor it represents)
58-
auto x = lhs->id();
59-
auto y = rhs->id();
60-
if (x == y)
61-
return lhs->node().compare(rhs->node());
62-
return static_cast<intptr_t>(x) - static_cast<intptr_t>(y);
33+
intptr_t actor_addr::compare(const abstract_actor* other) const noexcept {
34+
if (other) {
35+
return detail::compare(*this, other->ctrl());
36+
}
37+
return detail::compare(*this, nullptr);
6338
}
6439

65-
intptr_t actor_addr::compare(const actor_addr& other) const noexcept {
66-
return compare(ptr_.ctrl(), other.ptr_.ctrl());
40+
intptr_t actor_addr::compare(const actor_control_block* other) const noexcept {
41+
return detail::compare(*this, other);
6742
}
6843

69-
intptr_t actor_addr::compare(const abstract_actor* other) const noexcept {
70-
return compare(ptr_.ctrl(), other != nullptr ? other->ctrl() : nullptr);
44+
void actor_addr::swap(actor_addr& other) noexcept {
45+
using std::swap;
46+
swap(id_, other.id_);
47+
swap(node_, other.node_);
7148
}
7249

73-
intptr_t actor_addr::compare(const actor_control_block* other) const noexcept {
74-
return compare(ptr_.ctrl(), other);
50+
std::string to_string(const actor_addr& x) {
51+
std::string result;
52+
append_to_string(result, x);
53+
return result;
7554
}
7655

77-
void actor_addr::swap(actor_addr& other) noexcept {
78-
ptr_.swap(other.ptr_);
56+
void append_to_string(std::string& dst, const actor_addr& addr) {
57+
if (addr.id() == 0) {
58+
dst += "null";
59+
return;
60+
}
61+
using namespace std::literals;
62+
auto out = std::back_inserter(dst);
63+
const detail::simple_formatter<node_id> prefix;
64+
out = prefix.format(addr.node(), out);
65+
const auto infix = "/actor/id/"sv;
66+
detail::print_iterator_adapter buf{out};
67+
buf.insert(buf.end(), infix.begin(), infix.end());
68+
print(buf, addr.id());
7969
}
8070

8171
} // namespace caf

0 commit comments

Comments
 (0)