Skip to content

Commit d83313b

Browse files
committed
A new way of holding disp_binder inside agent.
Squashed commit of the following: commit 81d2553 Author: Yauheni Akhotnikau <[email protected]> Date: Wed May 31 15:52:41 2023 +0300 Some documentation related FIXME resolved. commit 7f9b4fb Author: Yauheni Akhotnikau <[email protected]> Date: Wed May 31 15:23:41 2023 +0300 More exception safe implementation of coop_impl_t::destroy_content. commit 5cc2ffc Author: Yauheni Akhotnikau <[email protected]> Date: Wed May 31 15:08:46 2023 +0300 disp_binder pointer is now stored inside the agent.
1 parent 72e8cd2 commit d83313b

File tree

6 files changed

+176
-62
lines changed

6 files changed

+176
-62
lines changed

dev/so_5/agent.hpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636
#include <so_5/message_handler_format_detector.hpp>
3737
#include <so_5/coop_handle.hpp>
3838

39+
#include <so_5/disp_binder.hpp>
40+
3941
#include <atomic>
4042
#include <map>
4143
#include <memory>
@@ -2669,6 +2671,24 @@ class SO_5_TYPE agent_t
26692671
*/
26702672
const priority_t m_priority;
26712673

2674+
/*!
2675+
* \brief Binder for this agent.
2676+
*
2677+
* Since v.5.7.5 disp_binder for the agent is stored inside the agent.
2678+
* It guarantees that disp_binder will be deleted after destruction
2679+
* of the agent (if there is no circular references between the agent
2680+
* and the disp_binder).
2681+
*
2682+
* This value will be set by coop_t when agent is being add to the
2683+
* coop.
2684+
*
2685+
* \note
2686+
* Access to that field provided by so_5::impl::internal_agent_iface_t.
2687+
*
2688+
* \since v.5.7.5
2689+
*/
2690+
disp_binder_shptr_t m_disp_binder;
2691+
26722692
//! Destroy all agent's subscriptions.
26732693
/*!
26742694
* \note

dev/so_5/coop.cpp

Lines changed: 58 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <so_5/impl/agent_ptr_compare.hpp>
1010

1111
#include <so_5/details/abort_on_fatal_error.hpp>
12+
#include <so_5/details/rollback_on_exception.hpp>
1213

1314
#include <so_5/exception.hpp>
1415
#include <so_5/environment.hpp>
@@ -54,35 +55,35 @@ void
5455
coop_impl_t::destroy_content(
5556
coop_t & coop ) noexcept
5657
{
57-
using std::swap;
58-
5958
// Initiate deleting of agents by hand to guarantee that
6059
// agents will be destroyed before return from coop_t
6160
// destructor.
6261
//
6362
// NOTE: because agents are stored here by smart references
6463
// for some agents this operation will lead only to reference
6564
// counter descrement. Not to deletion of agent.
66-
decltype(coop.m_agent_array) agents;
67-
swap( coop.m_agent_array, agents );
68-
69-
agents.clear();
65+
coop.m_agent_array.clear();
7066

7167
// Now all user resources should be destroyed.
72-
decltype(coop.m_resource_deleters) resources;
73-
swap( coop.m_resource_deleters, resources );
74-
75-
for( auto & d : resources )
68+
for( auto & d : coop.m_resource_deleters )
7669
d();
70+
coop.m_resource_deleters.clear();
7771
}
7872

7973
void
8074
coop_impl_t::do_add_agent(
8175
coop_t & coop,
8276
agent_ref_t agent_ref )
8377
{
84-
coop.m_agent_array.emplace_back(
85-
std::move(agent_ref), coop.m_coop_disp_binder );
78+
internal_agent_iface_t agent_iface{ *agent_ref };
79+
so_5::details::do_with_rollback_on_exception(
80+
[&]() {
81+
agent_iface.set_disp_binder( coop.m_coop_disp_binder );
82+
coop.m_agent_array.emplace_back( std::move(agent_ref) );
83+
},
84+
[&agent_iface]() noexcept {
85+
agent_iface.drop_disp_binder();
86+
} );
8687
}
8788

8889
void
@@ -91,8 +92,15 @@ coop_impl_t::do_add_agent(
9192
agent_ref_t agent_ref,
9293
disp_binder_shptr_t disp_binder )
9394
{
94-
coop.m_agent_array.emplace_back(
95-
std::move(agent_ref), std::move(disp_binder) );
95+
internal_agent_iface_t agent_iface{ *agent_ref };
96+
so_5::details::do_with_rollback_on_exception(
97+
[&]() {
98+
agent_iface.set_disp_binder( std::move(disp_binder) );
99+
coop.m_agent_array.emplace_back( std::move(agent_ref) );
100+
},
101+
[&agent_iface]() noexcept {
102+
agent_iface.drop_disp_binder();
103+
} );
96104
}
97105

98106
namespace
@@ -261,16 +269,17 @@ class coop_impl_t::registration_performer_t
261269
std::begin(m_coop.m_agent_array),
262270
std::end(m_coop.m_agent_array),
263271
[]( const auto & a, const auto & b ) noexcept {
264-
return special_agent_ptr_compare(
265-
*a.m_agent_ref, *b.m_agent_ref );
272+
return special_agent_ptr_compare( *a, *b );
266273
} );
267274
}
268275

269276
void
270277
bind_agents_to_coop()
271278
{
272-
for( auto & i : m_coop.m_agent_array )
273-
internal_agent_iface_t{ *i.m_agent_ref }.bind_to_coop( m_coop );
279+
for( const auto & agent_ref : m_coop.m_agent_array )
280+
{
281+
internal_agent_iface_t{ *agent_ref }.bind_to_coop( m_coop );
282+
}
274283
}
275284

276285
void
@@ -285,8 +294,10 @@ class coop_impl_t::registration_performer_t
285294
it != m_coop.m_agent_array.end();
286295
++it )
287296
{
288-
it->m_binder->preallocate_resources(
289-
*(it->m_agent_ref) );
297+
agent_t & agent = **it;
298+
internal_agent_iface_t agent_iface{ agent };
299+
agent_iface.query_disp_binder()
300+
.preallocate_resources( agent );
290301
}
291302
}
292303
catch( const std::exception & x )
@@ -296,8 +307,10 @@ class coop_impl_t::registration_performer_t
296307
it2 != it;
297308
++it2 )
298309
{
299-
it2->m_binder->undo_preallocation(
300-
*(it2->m_agent_ref) );
310+
agent_t & agent = **it2;
311+
internal_agent_iface_t agent_iface{ agent };
312+
agent_iface.query_disp_binder()
313+
.undo_preallocation( agent );
301314
}
302315

303316
SO_5_THROW_EXCEPTION(
@@ -314,8 +327,8 @@ class coop_impl_t::registration_performer_t
314327
{
315328
try
316329
{
317-
for( auto & info : m_coop.m_agent_array )
318-
internal_agent_iface_t{ *info.m_agent_ref }
330+
for( const auto & agent_ref : m_coop.m_agent_array )
331+
internal_agent_iface_t{ *agent_ref }
319332
.initiate_agent_definition();
320333
}
321334
catch( const exception_t & )
@@ -347,15 +360,22 @@ class coop_impl_t::registration_performer_t
347360
void
348361
bind_agents_to_disp() noexcept
349362
{
350-
for( auto & info : m_coop.m_agent_array )
351-
info.m_binder->bind( *info.m_agent_ref );
363+
for( const auto & agent_ref : m_coop.m_agent_array )
364+
{
365+
internal_agent_iface_t agent_iface{ *agent_ref };
366+
agent_iface.query_disp_binder().bind( *agent_ref );
367+
}
352368
}
353369

354370
void
355371
deallocate_disp_resources() noexcept
356372
{
357-
for( auto & info : m_coop.m_agent_array )
358-
info.m_binder->undo_preallocation( *(info.m_agent_ref) );
373+
for( const auto & agent_ref : m_coop.m_agent_array )
374+
{
375+
internal_agent_iface_t agent_iface{ *agent_ref };
376+
agent_iface.query_disp_binder()
377+
.undo_preallocation( *agent_ref );
378+
}
359379
}
360380

361381
public :
@@ -422,8 +442,10 @@ class coop_impl_t::deregistration_performer_t
422442
void
423443
shutdown_all_agents() noexcept
424444
{
425-
for( auto & info : m_coop.m_agent_array )
426-
internal_agent_iface_t{ *info.m_agent_ref }.shutdown_agent();
445+
for( const auto & agent_ref : m_coop.m_agent_array )
446+
{
447+
internal_agent_iface_t{ *agent_ref }.shutdown_agent();
448+
}
427449
}
428450

429451
void
@@ -475,8 +497,12 @@ coop_impl_t::do_final_deregistration_actions(
475497
coop_t & coop )
476498
{
477499
// Agents should be unbound from their dispatchers.
478-
for( auto & info : coop.m_agent_array )
479-
info.m_binder->unbind( *info.m_agent_ref );
500+
for( const auto & agent_ref : coop.m_agent_array )
501+
{
502+
agent_t & agent = *agent_ref;
503+
internal_agent_iface_t agent_iface{ agent };
504+
agent_iface.query_disp_binder().unbind( agent );
505+
}
480506

481507
// Now the coop can be removed from it's parent.
482508
// We don't except an exception here because m_parent should

dev/so_5/coop.hpp

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -933,37 +933,8 @@ class coop_t : public std::enable_shared_from_this<coop_t>
933933
}
934934

935935
protected:
936-
//! Information about agent and its dispatcher binding.
937-
/*!
938-
* \note
939-
* Order of fields in this structure is critically important:
940-
* the destructor of m_agent_ref must be invoked before the
941-
* destructor of m_binder.
942-
*/
943-
struct agent_with_disp_binder_t
944-
{
945-
agent_with_disp_binder_t(
946-
agent_ref_t agent_ref,
947-
disp_binder_shptr_t binder )
948-
: m_binder{ std::move(binder) }
949-
, m_agent_ref{ std::move(agent_ref) }
950-
{}
951-
952-
//! Binder to a dispatcher.
953-
/*!
954-
* \attention
955-
* This member has to be defined before m_agent_ref.
956-
* In that case the destructor of m_binder will be called
957-
* after the destructor of m_agent_ref.
958-
*/
959-
disp_binder_shptr_t m_binder;
960-
961-
//! Agent.
962-
agent_ref_t m_agent_ref;
963-
};
964-
965936
//! Typedef for the agent information container.
966-
using agent_array_t = std::vector< agent_with_disp_binder_t >;
937+
using agent_array_t = std::vector< agent_ref_t >;
967938

968939
/*!
969940
* \since

dev/so_5/disp_binder.hpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,15 @@ namespace so_5
2626
/*!
2727
* Dispatcher binders are used in the agent registration process to
2828
* binding of agents to desired dispatchers.
29+
*
30+
* \attention
31+
* If an implementation of disp_binder_t interface stores smart
32+
* pointers to agents in methods preallocate_resources() and bind()
33+
* then it must drop (or reset) these stored smart references in
34+
* undo_preallocation() and unbind() methods. Otherwise there will
35+
* be circular references between disp_binder and agents and this
36+
* will lead to memory leaks and other related problems (for
37+
* example, destructors for agents/disp_binders won't be called).
2938
*/
3039
class disp_binder_t
3140
: private std::enable_shared_from_this< disp_binder_t >

dev/so_5/impl/internal_agent_iface.hpp

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
#include <so_5/agent.hpp>
1616

17+
#include <so_5/ret_code.hpp>
18+
1719
namespace so_5 {
1820

1921
namespace impl {
@@ -54,6 +56,68 @@ class internal_agent_iface_t final
5456
{
5557
m_agent.shutdown_agent();
5658
}
59+
60+
/*!
61+
* \brief Setter for disp_binder.
62+
*
63+
* \attention
64+
* It's not a thread safe method. But it's expected that it will be used
65+
* in the context where just one entity owns the pointer to the agent.
66+
*
67+
* \since v.5.7.5
68+
*/
69+
void
70+
set_disp_binder( disp_binder_shptr_t binder )
71+
{
72+
if( m_agent.m_disp_binder )
73+
SO_5_THROW_EXCEPTION(
74+
rc_disp_binder_already_set_for_agent,
75+
"m_agent.m_disp_binder is not nullptr when "
76+
"set_disp_binder is called" );
77+
78+
m_agent.m_disp_binder = std::move(binder);
79+
}
80+
81+
/*!
82+
* \brief Getter for disp_binder.
83+
*
84+
* \attention
85+
* It's not a thread safe method. But it's expected that it will be used
86+
* in the context where just one entity owns the pointer to the agent.
87+
*
88+
* \since v.5.7.5
89+
*/
90+
[[nodiscard]]
91+
disp_binder_t &
92+
query_disp_binder() const
93+
{
94+
if( !m_agent.m_disp_binder )
95+
SO_5_THROW_EXCEPTION(
96+
rc_no_disp_binder_for_agent,
97+
"m_agent.m_disp_binder is nullptr when "
98+
"query_disp_binder is called" );
99+
100+
return *m_agent.m_disp_binder;
101+
}
102+
103+
/*!
104+
* \brief Helper method that drops pointer to disp_binder.
105+
*
106+
* This method is intended to be used for rollback actions
107+
* (for example, when disp_binder is set for the agent, but
108+
* agent can't be stored in a coop).
109+
*
110+
* \attention
111+
* It's not a thread safe method. But it's expected that it will be used
112+
* in the context where just one entity owns the pointer to the agent.
113+
*
114+
* \since v.5.7.5
115+
*/
116+
void
117+
drop_disp_binder() noexcept
118+
{
119+
m_agent.m_disp_binder.reset();
120+
}
57121
};
58122

59123
} /* namespace impl */

dev/so_5/ret_code.hpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,30 @@ const int rc_agent_deactivated = 189;
707707
*/
708708
const int rc_mpsc_mbox_expected = 190;
709709

710+
/*!
711+
* \brief The dispatcher binder is already set for the agent.
712+
*
713+
* Since v.5.7.5 disp_binder is stored inside the agent.
714+
* Only one disp_binder can be set for an agent. An attempt to set
715+
* disp_binder when the agent already has one is an error.
716+
*
717+
* \since v.5.7.5
718+
*/
719+
const int rc_disp_binder_already_set_for_agent = 194;
720+
721+
/*!
722+
* \brief The dispatcher binder is not set for the agent yet.
723+
*
724+
* Since v.5.7.5 disp_binder is stored inside the agent.
725+
* A disp_binder must be set for an agent during addition of the agent
726+
* to a coop. It means that if agent is added to coop the agent should have
727+
* non-null pointer to the disp_binder. If this pointer is still null then
728+
* it's an error.
729+
*
730+
* \since v.5.7.5
731+
*/
732+
const int rc_no_disp_binder_for_agent = 195;
733+
710734
//! \name Common error codes.
711735
//! \{
712736

0 commit comments

Comments
 (0)