apache / apache/dubbo-go

[Bug] time.After misuse: timer leak in ZK retry loop + dead 5s guard in accesslog (P1)

Open
#3,558 0 comments 0 reactions 1 assignee Claimed by @AlexStocks View on GitHub
☢️ Bug 3.3.3 need triage
Dominant language
Go
Stars
5k
Forks
1k
Avg merge
2d 8h
Merged PRs (30d)
31

Description

## Summary

Two `time.After` misuses cause either resource leakage or a dead timeout guard. `time.After(d)` creates a timer that is only released when the channel fires; using it inside a loop or with a `default` branch that returns immediately defeats the purpose and leaks timers/goroutines.

## Affected locations

### 1. Timer leak in ZooKeeper retry loop (P1)

`remoting/zookeeper/listener.go:270` and `:350`:

```go
for {
// ...
after := time.After(timeSecondDuration(failTimes * ConnDelay))
select {
case <-after:
// retry
}
}
```

A new timer is created on every iteration and is never `Stop()`-ed. On normal exit the timer only fires and is GC'd after the delay; under ZK flapping this accumulates a large number of live timers and goroutines.

**Fix:** use `t := time.NewTimer(d)` + `defer t.Stop()` (or reset in loop), outside/controlled by the select.

### 2. Dead 5-second guard in accesslog (P1)

`filter/accesslog/filter.go:216`:

```go
timeout := time.After(5 * time.Second)
for {
select {
case <-timeout:
// timeout path
default:
return // <- fires on the very first iteration
}
}
```

The `default` branch makes the loop `return` on the first iteration, so the 5s guard essentially never fires — log writes that would block are not actually guarded, and failures are silent.

**Fix:** remove the `default` (block on the select), or restructure to `select { case <-timeout: ... case <-done: return }`.

## Impact

- ZK listener: timer/goroutine accumulation on network jitter.
- accesslog: the intended 5s write timeout is a no-op; blocking log writes hang unnoticed.

## Verification

`GOTOOLCHAIN=local go vet ./...` on develop tip (HEAD 53d81d17) reports **zero** warnings (see #3552). `go vet`'s `lostcancel` does not flag `time.After` misuse — manual review / `noctx` + `staticcheck` (SA1015: using `time.After` in a loop) is needed.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.