Don't ignore CancelDownload() while a completion callback is pending

ABANDONED2026download
2026. 9. 8.Yelihi 프로필 이미지Yelihi
https://crrev.com/c/8277215

Chromium Background Download Service에서 다운로드 완료와 취소 사이에 발생하는 late callback 문제를 분석하고 개선을 시도한 기여

문제 설명

Chromium의 BackgroundDownloadService에서는 다운로드가 완료되면 ControllerImpl이 Client에 전달할 terminal callback을 PostTask를 통해 예약합니다.

문제는 callback이 예약된 시점과 실제 Client에 전달되는 시점 사이에 CancelDownload()가 호출될 수 있다는 점이었습니다.

대략적인 흐름은 다음과 같습니다.

DownloadDriver
    │
    │ OnDownloadSucceeded(guid)
    ▼
ControllerImpl
    │
    │ HandleCompleteDownload(SUCCEED)
    │
    │ PostTask(SendOnDownloadSucceeded)
    │
    │ TransitTo(COMPLETE)
    │
    │
    │ CancelDownload(guid)
    ▼
HandleCompleteDownload(CANCEL)
    │
    │ entry->state == COMPLETE
    │ → early return
    ▼
예약되어 있던 SendOnDownloadSucceeded 실행
    │
    ▼
Client::OnDownloadSucceeded()

즉 다운로드 성공 callback은 아직 Client에 전달되지 않았지만 내부 Entry는 이미 COMPLETE 상태가 됩니다.

이 사이 사용자가 다운로드를 취소하면 CancelDownload()가 호출되더라도 이미 완료 상태이기 때문에 취소 처리가 조기에 종료될 수 있습니다.

하지만 TaskQueue에 예약되어 있던 성공 callback은 사라지지 않기 때문에 이후 실행되어 Client에는 OnDownloadSucceeded()가 전달됩니다.

결과적으로 사용자가 취소했음에도 Client에서는 다운로드가 성공한 것처럼 처리될 수 있는 race condition이 존재했습니다.

해결 내용

1. 초기 해결안 — ControllerImpl에서 pending callback 상태 관리

처음에는 문제의 발생 지점인 ControllerImpl에서 아직 Client에 전달되지 않은 terminal callback을 명시적으로 관리하는 방향으로 접근했습니다.

이를 위해 두 종류의 GUID set을 추가했습니다.

// terminal callback이 예약되었지만 아직 Client에 전달되지 않은 GUID
guids_with_pending_completion_callback_;

// pending 상태에서 CancelDownload()가 호출된 GUID
guids_to_report_as_cancelled_;

다운로드가 완료되어 callback을 예약할 때 GUID를 pending set에 추가합니다.

개념적으로는 다음과 같은 구조입니다.

guids_with_pending_completion_callback_.insert(guid);

task_runner_->PostTask(
    FROM_HERE,
    base::BindOnce(
        &ControllerImpl::SendOnDownloadSucceeded,
        weak_ptr_factory_.GetWeakPtr(),
        guid));

이후 callback이 아직 pending인 상태에서 CancelDownload()가 호출되면 해당 GUID를 별도로 기록합니다.

if (guids_with_pending_completion_callback_.contains(guid)) {
  guids_to_report_as_cancelled_.insert(guid);
}

그리고 실제 terminal callback을 Client에 전달하는 시점에 취소 여부를 확인합니다.

guids_with_pending_completion_callback_.erase(guid);

if (guids_to_report_as_cancelled_.erase(guid)) {
  // 기존 success/failure 대신 CANCELLED를 전달
  // Client에는 terminal callback이 정확히 한 번 전달되도록 처리
}

이를 통해 다음과 같은 동작을 의도했습니다.

OnDownloadSucceeded(guid)
        │
        ▼
pending.insert(guid)
        │
        ▼
PostTask(SendOnDownloadSucceeded)
        │
        ▼
CancelDownload(guid)
        │
        ▼
pending.contains(guid) == true
        │
        ▼
cancelled.insert(guid)
        │
        ▼
SendOnDownloadSucceeded 실행
        │
        ├─ pending.erase(guid)
        └─ cancelled 확인
                 │
                 ▼
        OnDownloadFailed(CANCELLED)

즉 이미 예약된 callback 자체를 제거하는 대신, callback이 실제로 전달되는 순간 취소 상태를 확인하여 최종 결과를 결정하는 방식이었습니다.

2. 코드 리뷰 — 문제 해결 위치에 대한 재검토

하지만 리뷰 과정에서 중요한 피드백을 받았습니다.

"Can the client simply filter out or ignore late callbacks on their end after cancelling, rather than having ControllerImpl intercept and rewrite in-flight tasks? Background Fetch already does this."

핵심은 문제를 해결할 수 있느냐가 아니라 이 상태를 ControllerImpl이 관리하는 것이 올바른 책임 분리인가였습니다.

코드를 다시 추적해보니 Background Fetch Client는 자신이 시작한 다운로드에 대한 job과 GUID의 매핑을 이미 관리하고 있었습니다.

따라서 취소된 다운로드의 매핑을 제거한 뒤 callback을 받을 때 lookup guard를 두는 것만으로 late callback을 무시할 수 있었습니다.

개념적으로는 다음과 같은 형태입니다.

auto iter = download_job_id_map_.find(guid);

if (iter == download_job_id_map_.end())
  return;

Client는 다운로드의 시작과 취소를 모두 알고 있고 자신의 job과 GUID 사이의 관계도 이미 가지고 있습니다.

따라서 취소 이후 도착한 callback이라면 해당 GUID가 더 이상 Client의 map에 존재하지 않으므로 별도의 Service 상태 없이 무시할 수 있습니다.

이를 통해 처음 제안했던 다음과 같은 추가 상태 관리가 필요하지 않다는 결론에 도달했습니다.

pending completion GUID
cancelled while pending GUID
GUID reuse 관련 상태
callback 결과 재작성
callback 이후 cleanup

ControllerImpl에서 여러 상태를 추가해 Client의 의미를 추측하기보다, 해당 다운로드를 실제로 소유하고 있는 Client가 자신의 상태를 기준으로 callback을 받아들일지 결정하는 것이 더 단순하고 책임에도 맞는 구조였습니다.

3. 최종 처리

리뷰 의견을 바탕으로 기존 구현 방향을 다시 검토했습니다.

Background Fetch에서는 이미 Client의 lookup guard를 통해 취소 이후 도착한 late callback을 무시할 수 있었기 때문에 ControllerImpl에 별도의 상태를 추가할 필요가 없었습니다.

따라서 Service에 새로운 상태 관리 로직을 추가하기보다 기존 TODO를 제거하는 방향으로 이슈를 정리했습니다.

테스트 방법

초기 ControllerImpl 기반 해결안을 검증하기 위해 취소와 terminal callback 사이의 경계 상황을 다루는 단위 테스트를 작성했습니다.

주요 검증 대상은 다음과 같았습니다.

  • 성공 callback이 예약된 이후 취소되는 경우
  • 실패 callback이 예약된 이후 취소되는 경우
  • 취소 이후 기존 terminal callback이 전달되는 경우
  • terminal callback이 중복 전달되지 않는지 여부

특히 테스트에서는 callback이 즉시 실행되는 정상적인 흐름뿐만 아니라,

terminal event 발생
        ↓
callback PostTask
        ↓
CancelDownload()
        ↓
예약된 callback 실행

과 같이 callback 예약과 실행 사이에 cancel을 삽입하여 실제 문제가 발생하는 순서를 재현하는 것이 중요했습니다.

최종 리뷰 결과에서는 Service에 새로운 상태 관리 로직을 추가하지 않는 방향으로 정리되었기 때문에, 테스트 역시 단순히 구현 코드의 동작을 증명하는 것뿐 아니라 문제의 책임이 어느 계층에 있는지 판단하는 근거가 되었습니다.

배운 점

이번 기여에서 가장 크게 배운 부분은 문제를 해결하는 코드와 코드베이스에 적합한 해결책은 다를 수 있다는 점이었습니다.

처음에는 ControllerImpl에서 발생하는 race condition처럼 보였기 때문에 해당 계층에서 상태를 추가하여 문제를 해결하려 했습니다. 실제로 GUID set을 이용하면 취소와 terminal callback 사이의 상태를 구분하고 원하는 동작을 구현할 수 있었습니다.

하지만 리뷰를 통해 먼저 확인해야 했던 것은 새로운 상태가 필요한지가 아니라 그 상태를 누가 알고 있어야 하는가였습니다.

Client는 이미 다음 정보를 가지고 있었습니다.

Client
 ├─ 다운로드를 시작함
 ├─ CancelDownload()를 호출함
 └─ 자신의 job ↔ GUID 매핑을 관리함

반면 Service가 동일한 문제를 처리하려면 Client의 의도를 추측하기 위한 추가 상태가 필요했습니다.

Service
 ├─ pending callback인가?
 ├─ pending 상태에서 cancel되었는가?
 ├─ callback 결과를 변경해야 하는가?
 └─ 언제 상태를 정리해야 하는가?

결과적으로 새로운 상태를 추가하기 전에 이미 필요한 정보를 가지고 있는 계층이 어디인지 확인하는 것이 더 중요하다는 것을 배웠습니다.

또한 이번 작업에서는 AI를 이용해 DownloadService의 구조와 호출 관계를 빠르게 탐색했습니다. 특정 함수나 모듈을 기준으로 상위 호출자를 역추적하면서 전체 구조를 파악하는 데에는 효과적이었습니다.

하지만 AI는 제가 세운 "ControllerImpl에서 해결해야 한다"는 전제를 따라가면서 해당 해결안을 계속 구체화하는 경향도 보였습니다.

그래서 복잡한 코드베이스를 분석할 때는 AI에게 해결안을 정당화하도록 요청하기보다 다음을 먼저 확인하는 방식이 더 적절하다는 것을 경험했습니다.

  1. 현재 해결안의 반례가 있는가?
  2. 이 상태를 이미 알고 있는 다른 계층이 있는가?
  3. 이 책임이 정말 현재 모듈에 있어야 하는가?
  4. 기존 코드에는 같은 문제를 해결하는 패턴이 없는가?
  5. AI의 설명을 실제 호출 관계와 인자 흐름에서 확인할 수 있는가?

결국 AI는 코드베이스에 존재하는 맥락의 차이를 없애주는 도구라기보다, 그 맥락을 직접 찾아가고 학습하는 속도를 높여주는 도구로 사용하는 것이 적절하다는 점을 배웠습니다.

참고 자료

모든 컨트리뷰션 보기