MBL
Go / avito-start-code-quality / Задание 3: coverage, race detector и goleak
Go сложный

Задание 3: coverage, race detector и goleak

testingcoveragerace-detectorgoroutine-leakconcurrency

Важная оговорка сразу: в этом прогоне задания выполнен только первый пункт — отчёт о покрытии. Гонка данных на processed и утечка горутины в reportProgress намеренно оставлены как есть, без исправления — processor.go не менялся. Ниже подробно разобрано, что это за проблемы и почему их обнаруживают именно race и leak, но конкретный фикс здесь не приведён.

Что измеряет coverage и чего он не измеряет

make coverage
go tool cover -html=coverage.out -o coverage.html

go test -coverprofile=coverage.out инструментирует код при компиляции: каждый блок операторов получает счётчик, который увеличивается при выполнении. go tool cover -func печатает процент выполненных строк (точнее — блоков операторов) на функцию, -html рисует ту же статистику прямо поверх исходного кода зелёным/красным.

На processor.go эта команда даёт 96.4% покрытия: почти весь код выполняется хотя бы раз во время теста TestTotal. Это число ничего не говорит о двух главных проблемах файла — гонке на processed и утечке в reportProgress. Оба фрагмента кода выполняются каждый раз, когда тест их вызывает, значит оба посчитаны как «покрытые».

Покрытие строк отвечает на вопрос «этот код вообще запускался?», а не «этот код запускался во всех проблемных сценариях и с правильным результатом?» — гонка данных и утечка горутины оба относятся ко второй категории вопросов, которую line coverage принципиально не видит.

Гонка данных на processed — что происходит и почему её не видно на глаз

func Total(ctx context.Context, values []int) int {
    // ...
    var total atomic.Int64
    processed := 0
    var workers sync.WaitGroup

    for _, value := range values {
        workers.Add(1)
        go func() {
            defer workers.Done()
            total.Add(int64(value))
            processed++
        }()
    }

    workers.Wait()
    _ = processed
    return int(total.Load())
}

total защищена корректно — atomic.Int64 гарантирует, что параллельные Add не потеряют друг друга. processed++ — обычная переменная int, и её инкрементируют одновременно столько горутин, сколько элементов в values. processed++ на уровне процессора — это не одна атомарная операция, а последовательность «прочитать значение → прибавить 1 → записать обратно». Если две горутины выполнят чтение почти одновременно, обе прочитают одно и то же старое значение, обе прибавят единицу к нему и обе запишут один и тот же результат — фактически один из двух инкрементов теряется.

Гонка данных — это именно этот сценарий. В коде задания её последствия почти незаметны: processed нигде не читается после workers.Wait() (строка _ = processed буквально глушит компиляторное предупреждение о неиспользуемой переменной), поэтому потерянные инкременты ни на что не влияют и TestTotal спокойно проходит. Это и есть главная опасность гонок данных — они не обязаны ломать видимый результат каждый раз, поэтому обычный go test без -race их не ловит вообще.

Минимальная версия той же гонки, запускаемая прямо здесь с настоящим детектором (не эмуляция — go run -race в изолированной песочнице):

локально, go run -race

Запусти несколько раз: processed может напечататься как 100, а может — меньше (например, 97 или 99), в зависимости от того, как планировщик перемешал горутины в конкретном запуске. Именно эта недетерминированность и делает гонки данных особенно коварными в проде — код может месяцами отрабатывать «случайно правильно» и сломаться только под нагрузкой, когда горутин одновременно много.

make race (go test -race ./03-quality-checks) компилирует тест с включённым детектором гонок, который отслеживает все обращения к памяти и явно указывает на конфликт с точным местом в коде (processed++ в одной горутине и в другой) — в отличие от playground-демонстрации выше, реальный вывод -race называет конкретные номера строк и стек вызовов каждой из конфликтующих горутин.

Стандартный способ починить такую гонку — тот же приём, что уже применён к total: заменить processed int на atomic.Int64 (или защитить инкремент мьютексом) — в этом решении такая правка не сделана намеренно.

Утечка горутины в reportProgress

func Total(ctx context.Context, values []int) int {
    // ...
    go reportProgress(ctx)
    // ...
}

func reportProgress(ctx context.Context) {
    _ = ctx
    ticker := time.NewTicker(10 * time.Millisecond)
    for range ticker.C {
        // Здесь могла бы публиковаться метрика о ходе обработки.
    }
}

Total запускает reportProgress в отдельной горутине и передаёт ей ctx — но сама функция тут же присваивает ctx в _, то есть демонстративно его не использует. Цикл for range ticker.C не имеет условия выхода: он читает из тикера бесконечно, пока сам тикер не будет остановлен вызовом ticker.Stop() — а этого нигде не происходит. Когда Total завершается и возвращает результат, горутина reportProgress продолжает работать — она не связана с временем жизни Total никаким механизмом, кроме общего имени переменной ctx, которое ничего не значит для планировщика.

Утечка горутины — это именно такой случай. Каждый вызов Total оставляет позади ещё одну никогда не завершающуюся горутину с тикером внутри. За один вызов в тесте это не страшно, но в реальном сервисе, где Total могла бы вызываться на каждый HTTP-запрос, число таких зависших горутин росло бы без остановки — рантайм Go не умеет автоматически завершать «забытые» горутины, в отличие от сборщика мусора для памяти.

//go:build leaktest

func TestNoGoroutineLeaks(t *testing.T) {
    defer goleak.VerifyNone(t)

    ctx, cancel := context.WithCancel(context.Background())
    Total(ctx, []int{1, 2, 3})
    cancel()
}

goleak.VerifyNone(t) не читает исходный код и не анализирует логику статически — она снимает список работающих горутин до теста, даёт тесту отработать, затем снимает список после и сравнивает. Любая горутина, которая была создана тестом и всё ещё жива к моменту сравнения (за вычетом короткого таймаута на затухание), считается утечкой, и тест падает с полным дампом её стека — в данном случае это укажет прямо на reportProgress, застрявшую в for range ticker.C.

Обратите внимание: тест собран с build-тегом leaktest (//go:build leaktest) и не входит в обычный go test ./... — поэтому он отдельно запускается через make leak (go test -tags=leaktest ./03-quality-checks), а не выполняется каждый раз молча в фоне. cancel() после Total в тесте — намеренная подсказка: если бы reportProgress действительно слушала ctx.Done() и завершала цикл по отмене контекста, cancel() после вызова Total должен был бы дать горутине шанс корректно остановиться до того, как goleak снимет финальный список.

Правка, которую подразумевает задание (не выполнена в этом решении) — переписать цикл в reportProgress через select с веткой <-ctx.Done() и ticker.Stop() при выходе, чтобы горутина реально завершалась вместе с отменой контекста, а не жила вечно.

Что реально запущено в этом решении

make coverage
ok      code-quality-practice/03-quality-checks 0.004s  coverage: 96.4% of statements
code-quality-practice/03-quality-checks/processor.go:11:    Total       96.0%
code-quality-practice/03-quality-checks/processor.go:36:    reportProgress  100.0%
total:                              (statements)    96.4%

make race и make leak в этом прогоне не запускались с целью исправления — при попытке их выполнить прямо сейчас race действительно укажет на processed++, а leak (при отдельной сборке с -tags=leaktest) — на зависшую reportProgress, ровно как разобрано выше.

Самопроверка 0 / 4
Понимаю, почему 96% покрытия строк ничего не говорит о наличии гонки данных в этом же коде
Могу объяснить, что именно ловит флаг -race — не любой баг конкурентности, а конкретно неконтролируемый параллельный доступ к памяти
Понимаю, как goleak обнаруживает утечку — сравнением стека горутин до и после теста, а не статическим анализом кода
Знаю, что в этом решении гонка и утечка НЕ исправлены осознанно — только пункт 1 (coverage)
Как усвоено?