[Bug] Crash risks: os.Exit(0) in library code + P2C load balancer nil-interface panic (P0/P1)
- Dominant language
- Go
- Stars
- 5k
- Forks
- 1k
- Avg merge
- 2d 8h
- Merged PRs (30d)
- 31
Description
## Summary
Two distinct crash/hard-exit risks that can take down the whole process or a load-balancing goroutine:
## 1. `os.Exit` inside library code (P0)
`graceful_shutdown/shutdown.go:116` and `:127`:
```go
// inside the signal-handling goroutine of a core library (not main/cmd)
os.Exit(0)
```
`graceful_shutdown` is a **core library** embedded by applications, not an `main`/`cmd` entrypoint. Calling `os.Exit(0)` from a signal goroutine forcibly kills the process and bypasses any application-level graceful-shutdown hooks, deferred cleanups, and embedder control. An embedder cannot intercept or customize the exit.
**Fix:** signal completion via a channel / callback / returned error instead of `os.Exit`, and let the application's `main` decide when to exit.
## 2. P2C load balancer nil-interface panic (P1)
`cluster/loadbalance/p2c/loadbalance.go:121-153`:
```go
remainingIIface, err := m.GetMethodMetrics(...)
// only checks err != nil, NOT remainingIIface == nil
remainingI, ok := remainingIIface.(uint64) // if backend returned (nil, nil) -> ok=false
// subsequent code assumes the assertion succeeded -> panic:
// "interface conversion: interface is nil, not uint64"
```
If a metrics backend returns `(nil, nil)` (no error, no value), the code falls through to a type assertion on a nil interface and panics, crashing the P2C load-balancing path.
**Fix:** check `remainingIIface == nil` (and `!ok`) before use, and handle the missing-metrics case gracefully.
## Impact
- `os.Exit`: hard process kill breaks embedders' graceful shutdown.
- P2C: a specific metrics backend can crash the load balancer goroutine.
## Verification
`GOTOOLCHAIN=local go vet ./...` on develop tip (HEAD 53d81d17) reports **zero** warnings (see #3552). Neither issue is detected by default `go vet` — needs manual review / `staticcheck` (SA1500 for os.Exit in libs is a convention, not a lint rule) + defensive nil checks.
Contributor guide
Assessment
This issue has not been assessed yet.