Skip to content

Commit

Permalink
Fix check-then-act race condition
Browse files Browse the repository at this point in the history
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
  • Loading branch information
vlad-ivanov-name committed Sep 30, 2023
1 parent b34454b commit 1586eab
Showing 1 changed file with 9 additions and 8 deletions.
17 changes: 9 additions & 8 deletions josh-proxy/src/bin/josh-proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,15 @@ async fn fetch_upstream(

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

let us = upstream_repo.clone();
let semaphore = service
.fetch_permits
.lock()?
.entry(us.clone())
.or_insert(Arc::new(tokio::sync::Semaphore::new(1)))
.clone();
let permit = semaphore.acquire().await;

let fetch_timer_ok = {
if let Some(last) = service.fetch_timers.read()?.get(&key) {
let since = std::time::Instant::now().duration_since(*last);
Expand Down Expand Up @@ -183,15 +192,7 @@ async fn fetch_upstream(
let br_path = service.repo_path.join("mirror");

let span = tracing::span!(tracing::Level::TRACE, "fetch worker");
let us = upstream_repo.clone();
let ru = remote_url.clone();
let semaphore = service
.fetch_permits
.lock()?
.entry(us.clone())
.or_insert(Arc::new(tokio::sync::Semaphore::new(1)))
.clone();
let permit = semaphore.acquire().await;
let task_remote_auth = remote_auth.clone();
let fetch_result = tokio::task::spawn_blocking(move || {
let _span_guard = span.enter();
Expand Down

0 comments on commit 1586eab

Please sign in to comment.