Skip to content

Commit d5cadf4

Browse files
Fix check-then-act race condition
The semaphore is aquired after the conditions for fetching are already evaluated. It means that multiple threads/runners can decide to fetch simultaneously and then wait on the same semaphore, even though only one fetch is needed. Therefore, fetch would happen more often than what is specified in fetch times, and requests can be slow under load. commit-id:a4cd58e6
1 parent 5286c88 commit d5cadf4

File tree

1 file changed

+9
-8
lines changed

1 file changed

+9
-8
lines changed

josh-proxy/src/bin/josh-proxy.rs

+9-8
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,15 @@ async fn fetch_upstream(
129129

130130
let refs_to_fetch: Vec<_> = refs_to_fetch.iter().map(|x| x.to_string()).collect();
131131

132+
let us = upstream_repo.clone();
133+
let semaphore = service
134+
.fetch_permits
135+
.lock()?
136+
.entry(us.clone())
137+
.or_insert(Arc::new(tokio::sync::Semaphore::new(1)))
138+
.clone();
139+
let permit = semaphore.acquire().await;
140+
132141
let fetch_timer_ok = {
133142
if let Some(last) = service.fetch_timers.read()?.get(&key) {
134143
let since = std::time::Instant::now().duration_since(*last);
@@ -183,15 +192,7 @@ async fn fetch_upstream(
183192
let br_path = service.repo_path.join("mirror");
184193

185194
let span = tracing::span!(tracing::Level::TRACE, "fetch worker");
186-
let us = upstream_repo.clone();
187195
let ru = remote_url.clone();
188-
let semaphore = service
189-
.fetch_permits
190-
.lock()?
191-
.entry(us.clone())
192-
.or_insert(Arc::new(tokio::sync::Semaphore::new(1)))
193-
.clone();
194-
let permit = semaphore.acquire().await;
195196
let task_remote_auth = remote_auth.clone();
196197
let fetch_result = tokio::task::spawn_blocking(move || {
197198
let _span_guard = span.enter();

0 commit comments

Comments
 (0)