Skip to content

Commit 1d355ec

Browse files
authored
Merge pull request #1692 from mozilla-services/fix/threadpool-metrics-STOR-201
fix: avoid underflow of the queued_tasks metric
2 parents e93bb88 + 10daab0 commit 1d355ec

File tree

1 file changed

+12
-9
lines changed

1 file changed

+12
-9
lines changed

syncserver-common/src/lib.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -130,18 +130,18 @@ impl BlockingThreadpool {
130130
T: Send + 'static,
131131
E: fmt::Debug + Send + InternalError + 'static,
132132
{
133-
self.spawned_tasks.fetch_add(1, Ordering::SeqCst);
133+
self.spawned_tasks.fetch_add(1, Ordering::Relaxed);
134134
// Ensure the counter's always decremented (whether the task completed,
135135
// was cancelled or panicked)
136136
scopeguard::defer! {
137-
self.spawned_tasks.fetch_sub(1, Ordering::SeqCst);
137+
self.spawned_tasks.fetch_sub(1, Ordering::Relaxed);
138138
}
139139

140140
let active_threads = Arc::clone(&self.active_threads);
141141
let f_with_metrics = move || {
142-
active_threads.fetch_add(1, Ordering::SeqCst);
142+
active_threads.fetch_add(1, Ordering::Relaxed);
143143
scopeguard::defer! {
144-
active_threads.fetch_sub(1, Ordering::SeqCst);
144+
active_threads.fetch_sub(1, Ordering::Relaxed);
145145
}
146146
f()
147147
};
@@ -154,11 +154,14 @@ impl BlockingThreadpool {
154154

155155
/// Return the pool's current metrics
156156
pub fn metrics(&self) -> BlockingThreadpoolMetrics {
157-
// active_threads is decremented on a separate thread so we need a
158-
// strong Ordering to ensure it's in sync w/ spawned_tasks (otherwise
159-
// it could underflow queued_tasks)
160-
let spawned_tasks = self.spawned_tasks.load(Ordering::SeqCst);
161-
let active_threads = self.active_threads.load(Ordering::SeqCst);
157+
let spawned_tasks = self.spawned_tasks.load(Ordering::Relaxed);
158+
// active_threads is decremented on a separate thread so there's no
159+
// Drop order guarantee of spawned_tasks decrementing before it does:
160+
// catch the case where active_threads is larger
161+
let active_threads = self
162+
.active_threads
163+
.load(Ordering::Relaxed)
164+
.min(spawned_tasks);
162165
BlockingThreadpoolMetrics {
163166
queued_tasks: spawned_tasks - active_threads,
164167
active_threads,

0 commit comments

Comments
 (0)