Skip to content

collectors: fix nil Logger panic in collector startup#131

Merged
djkazic merged 1 commit into
lightninglabs:masterfrom
ZZiigguurraatt:fix/nil-logger-panic-collector-startup
Jul 1, 2026
Merged

collectors: fix nil Logger panic in collector startup#131
djkazic merged 1 commit into
lightninglabs:masterfrom
ZZiigguurraatt:fix/nil-logger-panic-collector-startup

Conversation

@ZZiigguurraatt

Copy link
Copy Markdown
Contributor

NewChannelsCollector launched its cache-refresh goroutine at construction time, but the package-global Logger was only initialized later in PrometheusExporter.Start via initLogRotator. If refreshClosedChannelsCache failed before Start ran, the goroutine dereferenced a nil Logger and panicked.

Fix the initialization ordering: move initLogRotator to the top of NewPrometheusExporter so logging is ready before any collector is constructed and before any collector goroutine can run. The constructor now returns an error; a logging init failure is fatal and surfaces on stderr so it stays detectable instead of being silently swallowed.

Fixes #129.

@ZZiigguurraatt

ZZiigguurraatt commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

I'm trying to actually test this code that has been proposed.

The example error in #129 was from the race condition described in "Two ways it triggers", probably hit because of so much crash looping because of "Permanent" trigger also mentioned there (which I was also getting sometimes).

I think "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." (also mentioned at "Two ways it triggers" in #129, the "Permanent" trigger) might actually be a bit wrong in how it actually fails (but it still is a permanent trigger).

Instead, when MkdirAll fails, it seems to crash completely right away (rather that waiting for a message to be attempted to written to the log), so we don't actually get the Logger assigned to nil (and let the rest of the process run until the first message fails to write to the log file).

With this fix, I'm only getting

failed to create log directory: mkdir /.lndmon: read-only file system

so that may hint that this fix is working (because I no longer hit the race condition), but not necessarily prove that the fix is working.

NewChannelsCollector launched its cache-refresh goroutine at
construction time, but the package-global Logger was only initialized
later in PrometheusExporter.Start via initLogRotator. If
refreshClosedChannelsCache failed before Start ran, the goroutine
dereferenced a nil Logger and panicked.

Fix the initialization ordering: move initLogRotator to the top of
NewPrometheusExporter so logging is ready before any collector is
constructed and before any collector goroutine can run. The constructor
now returns an error; a logging init failure is fatal and surfaces on
stderr so it stays detectable instead of being silently swallowed.

Fixes lightninglabs#129.
@ZZiigguurraatt ZZiigguurraatt force-pushed the fix/nil-logger-panic-collector-startup branch from c0c640d to 69d9a2d Compare July 1, 2026 16:28
@ZZiigguurraatt

Copy link
Copy Markdown
Contributor Author

signed commit

@djkazic djkazic merged commit bb991a5 into lightninglabs:master Jul 1, 2026
1 check passed
@ZZiigguurraatt ZZiigguurraatt deleted the fix/nil-logger-panic-collector-startup branch July 1, 2026 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants