apache / apache/dubbo-go

[Bug] Crash risks: os.Exit(0) in library code + P2C load balancer nil-interface panic (P0/P1)

Open
#3,559 0 comments 0 reactions 1 assignee Claimed by @AlexStocks View on GitHub
☢️ Bug 3.3.3 need triage protocol
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

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.