Skip to content

Commit 56fa721

Browse files
committed
Ensure cancellation::none implementation does not allocate
1 parent 7c6be8a commit 56fa721

1 file changed

Lines changed: 27 additions & 12 deletions

File tree

Source/Shared/arcana/threading/cancellation.h

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,11 @@
33
#include "arcana/containers/ticketed_collection.h"
44

55
#include <algorithm>
6-
#include <cassert>
76
#include <atomic>
8-
#include <memory>
9-
#include <vector>
10-
117
#include <functional>
8+
#include <cassert>
9+
#include <memory>
10+
#include <optional>
1211
#include <vector>
1312

1413
namespace arcana
@@ -25,7 +24,7 @@ namespace arcana
2524

2625
virtual bool cancelled() const = 0;
2726

28-
void throw_if_cancellation_requested()
27+
void throw_if_cancellation_requested() const
2928
{
3029
if (cancelled())
3130
{
@@ -60,22 +59,27 @@ namespace arcana
6059

6160
virtual ~cancellation()
6261
{
63-
assert(m_listeners.empty() && "you're destroying the listener collection and you still have listeners");
62+
assert((!m_listeners.has_value() || m_listeners->empty()) && "you're destroying the listener collection and you still have listeners");
6463
}
6564

6665
template<typename CallableT>
6766
ticket internal_add_listener(CallableT&& callback, std::function<void()>& copied)
6867
{
6968
std::lock_guard<std::mutex> guard{ m_mutex };
7069

70+
if (!m_listeners.has_value())
71+
{
72+
m_listeners.emplace();
73+
}
74+
7175
if (m_signaled)
7276
{
7377
copied = std::forward<CallableT>(callback);
74-
return m_listeners.insert(copied, m_mutex);
78+
return m_listeners->insert(copied, m_mutex);
7579
}
7680
else
7781
{
78-
return m_listeners.insert(std::forward<CallableT>(callback), m_mutex);
82+
return m_listeners->insert(std::forward<CallableT>(callback), m_mutex);
7983
}
8084
}
8185

@@ -86,8 +90,11 @@ namespace arcana
8690
{
8791
std::lock_guard<std::mutex> guard{ m_mutex };
8892

89-
listeners.reserve(m_listeners.size());
90-
std::copy(m_listeners.begin(), m_listeners.end(), std::back_inserter(listeners));
93+
if (m_listeners.has_value())
94+
{
95+
listeners.reserve(m_listeners->size());
96+
std::copy(m_listeners->begin(), m_listeners->end(), std::back_inserter(listeners));
97+
}
9198

9299
m_signaled = true;
93100
}
@@ -97,13 +104,21 @@ namespace arcana
97104
// then a child function does the same, the child
98105
// cancellation runs first. This avoids ownership
99106
// semantic issues.
100-
for(auto itr = listeners.rbegin(); itr != listeners.rend(); ++itr)
107+
for (auto itr = listeners.rbegin(); itr != listeners.rend(); ++itr)
108+
{
101109
(*itr)();
110+
}
102111
}
103112

104113
private:
114+
//
115+
105116
mutable std::mutex m_mutex;
106-
ticketed_collection<std::function<void()>> m_listeners;
117+
// std::optional is used here because the none() singleton is held in a no_destroy wrapper whose destructor
118+
// never runs. The underlying std::vector inside ticketed_collection allocates memory in debug builds on some
119+
// platforms, and that allocation would be a leak since no_destroy never frees it. Using std::optional will
120+
// delay the allocation until it's actually needed, which is never for the none() singleton.
121+
std::optional<ticketed_collection<std::function<void()>>> m_listeners;
107122
bool m_signaled = false;
108123
};
109124

0 commit comments

Comments
 (0)