Skip to content

Commit 7f1823c

Browse files
committed
Fix for bulk activation failures due to iterator invalidation (thanks to @bakaleks, Alexi Bakanov, Bridge Technologies, for the original report in PR #43)
(cherry picked from commit 3fb5925153db37fe44a8079da9aa89c2bb2a1e13)
1 parent 9c878d4 commit 7f1823c

2 files changed

Lines changed: 38 additions & 10 deletions

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#ifndef DETAIL_FOR_EACH_REVERSED_H
2+
#define DETAIL_FOR_EACH_REVERSED_H
3+
4+
#include <utility>
5+
6+
namespace detail
7+
{
8+
// for_each_reversed(first, last, f) is basically equivalent to
9+
// std::for_each(std::make_reverse_iterator(last), std::make_reverse_iterator(first), f)
10+
// but can be used when the given function object may invalidate the current iterator
11+
template <typename BidirectionalIterator, typename UnaryFunction>
12+
UnaryFunction for_each_reversed(BidirectionalIterator first, BidirectionalIterator last, UnaryFunction f)
13+
{
14+
if (first != last)
15+
{
16+
--last;
17+
while (first != last)
18+
{
19+
f(*last--);
20+
}
21+
f(*first);
22+
}
23+
return std::move(f);
24+
}
25+
}
26+
27+
#endif

Development/nmos-cpp-node/node_implementation.cpp

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#include "node_implementation.h"
22

3-
#include <boost/range/adaptor/reversed.hpp>
3+
#include "detail/for_each_reversed.h"
44
#include "nmos/activation_mode.h"
55
#include "nmos/connection_api.h"
66
#include "nmos/group_hint.h"
@@ -150,17 +150,18 @@ void node_implementation_thread(nmos::node_model& model, slog::base_gate& gate)
150150

151151
bool notify = false;
152152

153-
for (const auto& resource : by_updated | boost::adaptors::reversed)
153+
// since modify reorders the resource in this index, use for_each_reversed
154+
detail::for_each_reversed(by_updated.begin(), by_updated.end(), [&](const nmos::resource& resource)
154155
{
155-
if (!resource.has_data()) continue;
156+
if (!resource.has_data()) return;
156157

157158
const std::pair<nmos::id, nmos::type> id_type{ resource.id, resource.type };
158159

159160
auto& staged = nmos::fields::endpoint_staged(resource.data);
160161
auto& staged_activation = nmos::fields::activation(staged);
161162
auto& staged_mode_or_null = nmos::fields::mode(staged_activation);
162163

163-
if (staged_mode_or_null.is_null()) continue;
164+
if (staged_mode_or_null.is_null()) return;
164165

165166
const nmos::activation_mode staged_mode{ staged_mode_or_null.as_string() };
166167

@@ -181,22 +182,22 @@ void node_implementation_thread(nmos::node_model& model, slog::base_gate& gate)
181182
earliest_scheduled_activation = scheduled_activation;
182183
}
183184

184-
continue;
185+
return;
185186
}
186187
}
187188
else if (nmos::activation_modes::activate_immediate == staged_mode)
188189
{
189190
// check for cancelled in-flight immediate activation
190-
if (nmos::fields::requested_time(staged_activation).is_null()) continue;
191+
if (nmos::fields::requested_time(staged_activation).is_null()) return;
191192
// check for processed in-flight immediate activation
192-
if (!nmos::fields::activation_time(staged_activation).is_null()) continue;
193+
if (!nmos::fields::activation_time(staged_activation).is_null()) return;
193194

194195
slog::log<slog::severities::info>(gate, SLOG_FLF) << "Processing immediate activation for " << id_type;
195196
}
196197
else
197198
{
198199
slog::log<slog::severities::severe>(gate, SLOG_FLF) << "Unexpected activation mode for " << id_type;
199-
continue;
200+
return;
200201
}
201202

202203
const auto activation_time = nmos::tai_now();
@@ -229,7 +230,7 @@ void node_implementation_thread(nmos::node_model& model, slog::base_gate& gate)
229230
});
230231

231232
notify = true;
232-
}
233+
});
233234

234235
if ((nmos::tai_clock::time_point::max)() != earliest_scheduled_activation)
235236
{
@@ -254,4 +255,4 @@ void set_connection_sender_transportfile(nmos::resource& connection_sender, cons
254255
auto session_description = nmos::make_session_description(sdp_params, transport_params);
255256
auto sdp = utility::s2us(sdp::make_session_description(session_description));
256257
connection_sender.data[nmos::fields::endpoint_transportfile] = nmos::make_connection_sender_transportfile(sdp);
257-
}
258+
}

0 commit comments

Comments
 (0)