Panic: nil pointer deref in `ChannelsCollector` cache-refresh goroutine when `Logger` is uninitialized

[ZZiigguurraatt]

Affected versions: v0.2.15 (latest tag); code path unchanged on master.

Introduced in: 0a2570a ("collectors: tolerate DeadlineExceeded in all scrape-cycle
RPCs"), first released in v0.2.15.

Summary

lndmon can crash on startup with a nil pointer dereference in the ChannelsCollector
cache-refresh goroutine. The package-global Logger is used by a goroutine that is
launched before Logger is initialized, so any early collector error dereferences a
nil logger.

Commit 0a2570a changed several collector error paths from errChan <- err to a
log-then-check pattern (Logger.Errorf(...) / Logger.Error(...) followed by an
IsDeadlineExceeded guard). The intent was good — log transient DeadlineExceeded
errors and skip the scrape cycle instead of exiting — but it put a Logger use on code
that can run before Logger is assigned. The crash reported here is in
ChannelsCollector's refresh goroutine, but the same nil-Logger exposure now exists in
every collector touched by that commit: InfoCollector, ChannelsCollector,
WalletCollector, GraphCollector, and WtClientCollector. Any of them erroring while
Logger is nil crashes the same way.

Panic

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x58 pc=0x14a5944]

goroutine 30 [running]:
github.com/lightninglabs/lndmon/collectors.NewChannelsCollector.func1()
    /go/src/github.com/lightninglabs/lndmon/collectors/channels_collector.go:205 +0xe4
created by github.com/lightninglabs/lndmon/collectors.NewChannelsCollector in goroutine 1
    /go/src/github.com/lightninglabs/lndmon/collectors/channels_collector.go:198 +0x996

Root cause — use-before-init ordering

1. NewPrometheusExporter() constructs the collectors. NewChannelsCollector immediately
   launches a cache-refresh goroutine (collectors/channels_collector.go:198) whose first
   loop iteration calls refreshClosedChannelsCache() → ClosedChannels with no initial
   delay.
2. The error branch of that loop logs unconditionally (collectors/channels_collector.go:205):

   err := collector.refreshClosedChannelsCache()
   if err != nil {
       Logger.Errorf("ChannelsCollector refreshClosedChannelsCache failed with: %v", err)
       if !IsDeadlineExceeded(err) { errChan <- err }
   }

3. Logger is assigned only later, inside PrometheusExporter.Start() → initLogRotator()
   (collectors/prometheus.go:154), and only at the end of that function
   (collectors/log.go:55), after os.MkdirAll(logDir, 0700) (collectors/log.go:33)
   succeeds.

In lndmon.go's start(), NewPrometheusExporter(...) (line 81) runs before
exporter.Start() (line 84) — so the collector goroutine is live while Logger is still
nil.

Two ways it triggers

• Race: when lnd's RPC isn't fully ready yet, ClosedChannels errors in the window
  before Start() finishes logger init → Logger.Errorf on nil.
• Permanent: if initLogRotator's MkdirAll fails (e.g. running under a read-only root
  filesystem with the default logdir $HOME/.lndmon/logs unwritable), Start() returns
  early and Logger is never assigned — so it stays nil for the whole process, and any
  collector error (startup, or a later transient deadline exceeded) panics.

Reproduction

Run lndmon against an lnd node whose RPC isn't fully serving yet (or with an unwritable
--prometheus.logdir) so the first ClosedChannels scrape errors. The cache-refresh
goroutine panics instead of logging.

Suggested fixes (any one resolves it)

1. Initialize logging (initLogRotator) before constructing collectors, so Logger is
   non-nil before any goroutine starts.
2. Don't launch the cache-refresh goroutine in NewChannelsCollector (construction); start
   it from Start() after logger init.
3. Treat initLogRotator failure as fatal in Start() and ensure no collector goroutine
   runs before it — i.e. don't leave Logger nil while goroutines are live.
4. Defensively nil-guard the error path (or inject a non-nil default/no-op logger), so a
   logging failure can't escalate a recoverable scrape error into a crash.

Fix #1 or #2 is the cleanest. The Logger.Errorf / Logger.Error calls added in
0a2570a (alongside the IsDeadlineExceeded handling) sit on paths that can run before
Logger exists. Whatever fix is chosen should cover all five collectors changed in that
commit, not just ChannelsCollector.

[djkazic]

Nice find!

[djkazic]

#130