Skip to content

Commit 4a3a71b

Browse files
committed
Fix for possible threading problem.
The Queue implementation had a mechanism to know whether it was in use or in the process of being shut down using the m_done atomic bool. But this interfered with the externally used mechanism of putting special "end of data" markers in the queue which could lead to a situation where queue readers would not shutdown properly.
1 parent cc85925 commit 4a3a71b

File tree

2 files changed

+3
-13
lines changed

2 files changed

+3
-13
lines changed

include/osmium/thread/pool.hpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,6 @@ namespace osmium {
180180

181181
~Pool() {
182182
shutdown_all_workers();
183-
m_work_queue.shutdown();
184183
}
185184

186185
size_t queue_size() const {

include/osmium/thread/queue.hpp

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,6 @@ namespace osmium {
7373
/// Used to signal readers when data is available in the queue.
7474
std::condition_variable m_data_available;
7575

76-
std::atomic<bool> m_done;
77-
7876
#ifdef OSMIUM_DEBUG_QUEUE_SIZE
7977
/// The largest size the queue has been so far.
8078
size_t m_largest_size;
@@ -111,8 +109,7 @@ namespace osmium {
111109
m_name(name),
112110
m_mutex(),
113111
m_queue(),
114-
m_data_available(),
115-
m_done(false)
112+
m_data_available()
116113
#ifdef OSMIUM_DEBUG_QUEUE_SIZE
117114
,
118115
m_largest_size(0),
@@ -125,7 +122,6 @@ namespace osmium {
125122
}
126123

127124
~Queue() {
128-
shutdown();
129125
#ifdef OSMIUM_DEBUG_QUEUE_SIZE
130126
std::cerr << "queue '" << m_name << "' with max_size=" << m_max_size << " had largest size " << m_largest_size << " and was full " << m_full_counter << " times in " << m_push_counter << " push() calls and was empty " << m_empty_counter << " times in " << m_pop_counter << " pop() calls\n";
131127
#endif
@@ -157,11 +153,6 @@ namespace osmium {
157153
m_data_available.notify_one();
158154
}
159155

160-
void shutdown() {
161-
m_done = true;
162-
m_data_available.notify_all();
163-
}
164-
165156
void wait_and_pop(T& value) {
166157
#ifdef OSMIUM_DEBUG_QUEUE_SIZE
167158
++m_pop_counter;
@@ -173,7 +164,7 @@ namespace osmium {
173164
}
174165
#endif
175166
m_data_available.wait(lock, [this] {
176-
return !m_queue.empty() || m_done;
167+
return !m_queue.empty();
177168
});
178169
if (!m_queue.empty()) {
179170
value = std::move(m_queue.front());
@@ -192,7 +183,7 @@ namespace osmium {
192183
}
193184
#endif
194185
if (!m_data_available.wait_for(lock, std::chrono::seconds(1), [this] {
195-
return !m_queue.empty() || m_done;
186+
return !m_queue.empty();
196187
})) {
197188
return;
198189
}

0 commit comments

Comments
 (0)