Skip to content

fix: guard nil DefaultLogger.Out in moduleReload teardown (#846)#849

Open
SAY-5 wants to merge 1 commit into
foxcpp:devfrom
SAY-5:fix-reload-nil-logger
Open

fix: guard nil DefaultLogger.Out in moduleReload teardown (#846)#849
SAY-5 wants to merge 1 commit into
foxcpp:devfrom
SAY-5:fix-reload-nil-logger

Conversation

@SAY-5
Copy link
Copy Markdown

@SAY-5 SAY-5 commented May 21, 2026

Fixes #846.

systemctl reload maddy (SIGUSR2) crashes the daemon with a nil pointer dereference in the async teardown goroutine of moduleReload. The new server has already bound listeners, but the panicking goroutine takes the whole process down with it, leaving the host with no mail service running until systemctl restart maddy is issued.

Root cause: when the configuration has no top-level log directive, defaultLogOutput returns (nil, nil) and c.DefaultLogger.Out is left unset. The moduleMain shutdown path at maddy.go:452 already nil-checks before calling Close(); the reload teardown goroutine added in 5841e95 (log: Refactor to define proper loggers tree) did not, so the close at maddy.go:520 panics.

Fix: factor the close into a small closeContainerLogOutput helper that mirrors the existing nil-safe shutdown pattern, and call it from the reload teardown goroutine.

Reproducer (boots a container with no log directive and runs the same Out.Close() call that the teardown goroutine made):

c := &container.C{DefaultLogger: &log.Logger{}} // Out is nil
_ = c.DefaultLogger.Out.Close()
// panic: runtime error: invalid memory address or nil pointer dereference

Tests:

  • TestCloseContainerLogOutput_NilOut — exercises the bug condition; before this change the equivalent inline Out.Close() panicked.
  • TestCloseContainerLogOutput_NonNil — happy path is unchanged.
  • TestCloseContainerLogOutput_CloseError — Close errors are still surfaced through the new container's logger.
  • TestCloseContainerLogOutput_NilContainer — defensive coverage for nil container / nil logger.

go test -race . and gofmt -l . are clean.

When the configuration has no top-level 'log' directive, defaultLogOutput
returns (nil, nil) and the container's DefaultLogger.Out is left unset.
The async teardown goroutine in moduleReload called Out.Close()
unconditionally, so 'systemctl reload maddy' (SIGUSR2) panicked and the
daemon exited with status=2/INVALIDARGUMENT, dropping the mail service.

Mirror the nil-check used by the synchronous shutdown path in moduleMain
by extracting the close into a small helper and calling it from the
reload teardown goroutine. Add unit tests covering the nil-Out, non-nil,
Close-error, and nil-container code paths.

Fixes foxcpp#846.
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.

1 participant