From 1c1a01ae4b7ec6bd3a43f48fdfa00884db92b8e4 Mon Sep 17 00:00:00 2001 From: "fox.cpp" Date: Sat, 16 May 2026 01:24:05 +0300 Subject: [PATCH 1/7] msgpipeline: Ensure logger is always initialized for nested pipelines --- internal/msgpipeline/msgpipeline.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/msgpipeline/msgpipeline.go b/internal/msgpipeline/msgpipeline.go index 8fc9c54d..6025bf06 100644 --- a/internal/msgpipeline/msgpipeline.go +++ b/internal/msgpipeline/msgpipeline.go @@ -90,6 +90,7 @@ func New(globals map[string]interface{}, cfg []config.Node) (*MsgPipeline, error return &MsgPipeline{ msgpipelineCfg: parsedCfg, Resolver: dns.DefaultResolver(), + Log: log.DefaultLogger.Sublogger("msgpipeline"), }, err } From fd6d880141d880251fba84eeb0a171648506393a Mon Sep 17 00:00:00 2001 From: "fox.cpp" Date: Sat, 16 May 2026 02:06:40 +0300 Subject: [PATCH 2/7] target/remote: Fix dangling destination limiter on first RCPT error, add debug logs Might fix #842. --- internal/endpoint/smtp/smtp.go | 2 +- internal/limits/limits.go | 37 ++++++++++++++++++++++++--- internal/smtpconn/smtpconn.go | 2 +- internal/target/remote/connect.go | 6 +++++ internal/target/remote/remote.go | 3 ++- internal/target/remote/remote_test.go | 2 +- 6 files changed, 44 insertions(+), 8 deletions(-) diff --git a/internal/endpoint/smtp/smtp.go b/internal/endpoint/smtp/smtp.go index eb334556..acfcca3b 100644 --- a/internal/endpoint/smtp/smtp.go +++ b/internal/endpoint/smtp/smtp.go @@ -274,7 +274,7 @@ func (endp *Endpoint) setConfig(cfg *config.Map) error { cfg.Bool("defer_sender_reject", false, true, &endp.deferServerReject) cfg.Int("max_logged_rcpt_errors", false, false, 5, &endp.maxLoggedRcptErrors) cfg.Custom("limits", false, false, func() (interface{}, error) { - return &limits.Group{}, nil + return limits.Empty(endp.log.Sublogger("limits")), nil }, func(cfg *config.Map, n config.Node) (interface{}, error) { var g *limits.Group if err := modconfig.GroupFromNode("limits", n.Args, n, cfg.Globals, &g); err != nil { diff --git a/internal/limits/limits.go b/internal/limits/limits.go index eba335d4..66086f7a 100644 --- a/internal/limits/limits.go +++ b/internal/limits/limits.go @@ -28,18 +28,21 @@ package limits import ( "context" + "fmt" "net" "strconv" "time" "github.com/foxcpp/maddy/framework/config" "github.com/foxcpp/maddy/framework/container" + "github.com/foxcpp/maddy/framework/log" "github.com/foxcpp/maddy/framework/module" "github.com/foxcpp/maddy/framework/module/modules" "github.com/foxcpp/maddy/internal/limits/limiters" ) type Group struct { + log *log.Logger instName string global limiters.MultiLimit @@ -48,8 +51,15 @@ type Group struct { dest *limiters.BucketSet // BucketSet of MultiLimit } +func Empty(log *log.Logger) *Group { + return &Group{ + log: log, + } +} + func New(c *container.C, _, instName string) (module.Module, error) { return &Group{ + log: c.DefaultLogger.Sublogger("limits"), instName: instName, }, nil } @@ -177,22 +187,28 @@ func (g *Group) TakeMsg(ctx context.Context, addr net.IP, sourceDomain string) e ctx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() + g.log.DebugMsg("global TakeContext") if err := g.global.TakeContext(ctx); err != nil { - return err + return fmt.Errorf("TakeMsg: global: %w", err) } + g.log.DebugMsg("global TakeContext done") if g.ip != nil { + g.log.DebugMsg("ip TakeContext", "ip", addr.String()) if err := g.ip.TakeContext(ctx, addr.String()); err != nil { g.global.Release() - return err + return fmt.Errorf("TakeMsg: ip: %w", err) } + g.log.DebugMsg("ip TakeContext done", "ip", addr.String()) } if g.source != nil { + g.log.DebugMsg("source TakeContext", "domain", sourceDomain) if err := g.source.TakeContext(ctx, sourceDomain); err != nil { g.global.Release() g.ip.Release(addr.String()) - return err + return fmt.Errorf("TakeMSg: source: %w", err) } + g.log.DebugMsg("source TakeContext done", "domain", sourceDomain) } return nil } @@ -201,17 +217,27 @@ func (g *Group) TakeDest(ctx context.Context, domain string) error { if g.dest == nil { return nil } + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() - return g.dest.TakeContext(ctx, domain) + + g.log.DebugMsg("TakeDest", "domain", domain) + if err := g.dest.TakeContext(ctx, domain); err != nil { + return fmt.Errorf("TakeDest: dest: %w", err) + } + g.log.DebugMsg("TakeDest done", "domain", domain) + return nil } func (g *Group) ReleaseMsg(addr net.IP, sourceDomain string) { + g.log.DebugMsg("global ReleaseMsg") g.global.Release() if g.ip != nil { + g.log.DebugMsg("ip ReleaseMsg", "ip", addr.String()) g.ip.Release(addr.String()) } if g.source != nil { + g.log.DebugMsg("source ReleaseMsg", "domain", sourceDomain) g.source.Release(sourceDomain) } } @@ -220,7 +246,10 @@ func (g *Group) ReleaseDest(domain string) { if g.dest == nil { return } + + g.log.DebugMsg("ReleaseDest", "domain", domain) g.dest.Release(domain) + g.log.DebugMsg("ReleaseDest done", "domain", domain) } func (g *Group) Name() string { diff --git a/internal/smtpconn/smtpconn.go b/internal/smtpconn/smtpconn.go index 22398937..7ec93cac 100644 --- a/internal/smtpconn/smtpconn.go +++ b/internal/smtpconn/smtpconn.go @@ -239,7 +239,7 @@ func (c *C) attemptConnect(ctx context.Context, lmtp bool, endp config.Endpoint, conn, err = c.Dialer(dialCtx, endp.Network(), endp.Address()) cancel() if err != nil { - return false, nil, nil, err + return false, nil, nil, fmt.Errorf("dialer: %w", err) } if endp.IsTLS() { diff --git a/internal/target/remote/connect.go b/internal/target/remote/connect.go index 4456d927..3b9692d0 100644 --- a/internal/target/remote/connect.go +++ b/internal/target/remote/connect.go @@ -45,6 +45,7 @@ type mxConn struct { errored bool reuseLimit int + takeDest bool // Amount of times connection was used for an SMTP transaction. transactions int @@ -207,6 +208,10 @@ func (rd *remoteDelivery) attemptMX(ctx context.Context, conn *mxConn, record *n } func (rd *remoteDelivery) closeConn(c *mxConn) { + if c.takeDest { + rd.rt.limits.ReleaseDest(c.domain) + } + if err := c.Close(); err != nil { rd.log.Error("client connection close failed", err) } @@ -270,6 +275,7 @@ func (rd *remoteDelivery) connectionForDomain(ctx context.Context, domain string return nil, err } region.End() + conn.takeDest = true // Relaxed REQUIRETLS mode is not conforming to the specification strictly // but allows to start deploying client support for REQUIRETLS without the diff --git a/internal/target/remote/remote.go b/internal/target/remote/remote.go index 93b26573..daf5a874 100644 --- a/internal/target/remote/remote.go +++ b/internal/target/remote/remote.go @@ -130,7 +130,7 @@ func (rt *Target) Configure(inlineArgs []string, cfg *config.Map) error { return p.L, nil }, &rt.policies) cfg.Custom("limits", false, false, func() (interface{}, error) { - return &limits.Group{}, nil + return limits.Empty(rt.log.Sublogger("limits")), nil }, func(cfg *config.Map, n config.Node) (interface{}, error) { var g *limits.Group if err := modconfig.GroupFromNode("limits", n.Args, n, cfg.Globals, &g); err != nil { @@ -455,6 +455,7 @@ func (rd *remoteDelivery) Commit(ctx context.Context) error { func (rd *remoteDelivery) Close() error { for _, conn := range rd.connections { rd.rt.limits.ReleaseDest(conn.domain) + conn.takeDest = false conn.transactions++ if !conn.Usable() { diff --git a/internal/target/remote/remote_test.go b/internal/target/remote/remote_test.go index ca4fd656..06f0cb80 100644 --- a/internal/target/remote/remote_test.go +++ b/internal/target/remote/remote_test.go @@ -62,7 +62,7 @@ func testTarget(t *testing.T, zones map[string]mockdns.Zone, extResolver *dns.Ex tlsConfig: &tls.Config{}, log: testutils.Logger(t, "remote"), policies: extraPolicies, - limits: &limits.Group{}, + limits: limits.Empty(testutils.Logger(t, "limits")), pool: pool.New(pool.Config{ MaxKeys: 5000, MaxConnsPerKey: 5, // basically, max. amount of idle connections in cache From e42741c7577746a474a45a818d554d578f4f853d Mon Sep 17 00:00:00 2001 From: "fox.cpp" Date: Sat, 16 May 2026 15:59:40 +0300 Subject: [PATCH 3/7] ci: Re-enable arm64 docker builds --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e7155b68..6ec83b75 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -144,7 +144,7 @@ jobs: id: docker with: context: . - platforms: linux/amd64 #,linux/arm64 Temporary disabled due to SIGSEGV in gcc. + platforms: linux/amd64,linux/arm64 file: Dockerfile push: true tags: ${{ steps.meta.outputs.tags }} From 9c78f96902f931383dda12e6f8fdeade2262a010 Mon Sep 17 00:00:00 2001 From: oidq Date: Wed, 20 May 2026 18:42:10 +0200 Subject: [PATCH 4/7] fix: check for nil Out in Logger on Close() * add Logger.Close(), which safely closes underlying Out * should fix #846 --- framework/log/log.go | 9 +++++++++ maddy.go | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/framework/log/log.go b/framework/log/log.go index 3ad291a6..263732a0 100644 --- a/framework/log/log.go +++ b/framework/log/log.go @@ -205,6 +205,15 @@ func (l *Logger) Write(s []byte) (int, error) { return len(s), nil } +// Close closes underlying output in Out. +func (l *Logger) Close() error { + if l.Out == nil { + return nil + } + + return l.Out.Close() +} + // DebugWriter returns a writer that will act like Logger.Write // but will use debug flag on messages. If Logger.Debug is false, // Write method of returned object will be no-op. diff --git a/maddy.go b/maddy.go index c3a560bc..4d7a8179 100644 --- a/maddy.go +++ b/maddy.go @@ -450,7 +450,7 @@ func moduleMain(configPath string) error { c.DefaultLogger.Msg("server stopped") if c.DefaultLogger.Out != nil { - if err := c.DefaultLogger.Out.Close(); err != nil { + if err := c.DefaultLogger.Close(); err != nil { log.DefaultLogger.Error("failed to close output logger", err) } } @@ -517,7 +517,7 @@ func moduleReload(oldContainer *container.C, configPath string, asyncStopWg *syn oldContainer.DefaultLogger.Error("moduleStop failed", err) } oldContainer.DefaultLogger.Msg("old server stopped") - if err := oldContainer.DefaultLogger.Out.Close(); err != nil { + if err := oldContainer.DefaultLogger.Close(); err != nil { newContainer.DefaultLogger.Error("failed to close old server log", err) } From 6200c517c3b3ce95c5f2ba523fc9b911dbc2af51 Mon Sep 17 00:00:00 2001 From: oidq Date: Wed, 20 May 2026 18:48:47 +0200 Subject: [PATCH 5/7] fix(systemd): report READY=1 after reload SystemD would report maddy in "reloading (reload-notify)" state even after successful reload. It should send "READY=1" after finishing the reload. --- maddy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/maddy.go b/maddy.go index c3a560bc..952e8354 100644 --- a/maddy.go +++ b/maddy.go @@ -521,7 +521,7 @@ func moduleReload(oldContainer *container.C, configPath string, asyncStopWg *syn newContainer.DefaultLogger.Error("failed to close old server log", err) } - systemdStatus(SDReloading, "Configuration running.") + systemdStatus(SDReady, "Configuration running.") }() return newContainer From 632908a992a7c01f960d8f90fb1f04059f616ee3 Mon Sep 17 00:00:00 2001 From: SAY-5 Date: Thu, 21 May 2026 02:34:55 -0700 Subject: [PATCH 6/7] fix: guard nil DefaultLogger.Out in moduleReload teardown 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 #846. --- maddy.go | 21 +++++++-- maddy_test.go | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 3 deletions(-) create mode 100644 maddy_test.go diff --git a/maddy.go b/maddy.go index ba5a95ac..4a1733cb 100644 --- a/maddy.go +++ b/maddy.go @@ -462,6 +462,23 @@ func moduleMain(configPath string) error { return nil } +// closeContainerLogOutput closes c.DefaultLogger.Out if it is set, mirroring +// the nil-safe behaviour of the moduleMain shutdown path. When the +// configuration has no `log` directive, defaultLogOutput returns (nil, nil) +// and c.DefaultLogger.Out is left unset; closing it unconditionally panics. +// errLogger receives any close failure (it must not be the logger of c, +// which is being torn down). +func closeContainerLogOutput(c *container.C, errLogger *log.Logger) { + if c == nil || c.DefaultLogger == nil || c.DefaultLogger.Out == nil { + return + } + if err := c.DefaultLogger.Out.Close(); err != nil { + if errLogger != nil { + errLogger.Error("failed to close old server log", err) + } + } +} + func moduleReload(oldContainer *container.C, configPath string, asyncStopWg *sync.WaitGroup) *container.C { oldContainer.DefaultLogger.Msg("reloading server...") systemdStatus(SDReloading, "Reloading server...") @@ -521,9 +538,7 @@ func moduleReload(oldContainer *container.C, configPath string, asyncStopWg *syn oldContainer.DefaultLogger.Error("moduleStop failed", err) } oldContainer.DefaultLogger.Msg("old server stopped") - if err := oldContainer.DefaultLogger.Out.Close(); err != nil { - newContainer.DefaultLogger.Error("failed to close old server log", err) - } + closeContainerLogOutput(oldContainer, newContainer.DefaultLogger) systemdStatus(SDReloading, "Configuration running.") }() diff --git a/maddy_test.go b/maddy_test.go new file mode 100644 index 00000000..8d0289bd --- /dev/null +++ b/maddy_test.go @@ -0,0 +1,122 @@ +/* +Maddy Mail Server - Composable all-in-one email server. +Copyright © 2019-2020 Max Mazurov , Maddy Mail Server contributors + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see . +*/ + +package maddy + +import ( + "errors" + "testing" + "time" + + "github.com/foxcpp/maddy/framework/container" + "github.com/foxcpp/maddy/framework/log" +) + +// trackingOutput is a log.Output that records whether Close was called. +type trackingOutput struct { + closed bool + closeErr error +} + +func (o *trackingOutput) Write(_ time.Time, _ bool, _ string) {} +func (o *trackingOutput) Close() error { + o.closed = true + return o.closeErr +} + +func newErrLogger() (*log.Logger, *trackingOutput) { + out := &trackingOutput{} + return &log.Logger{Out: out}, out +} + +// TestCloseContainerLogOutput_NilOut verifies that closeContainerLogOutput +// does not panic when DefaultLogger.Out is nil, which is the configuration +// produced by defaultLogOutput when the user's maddy.conf has no `log` +// directive. Without the nil guard, SIGUSR2 reload panicked in the teardown +// goroutine of moduleReload and the daemon died (foxcpp/maddy#846). +func TestCloseContainerLogOutput_NilOut(t *testing.T) { + c := &container.C{ + DefaultLogger: &log.Logger{}, // Out is the zero value (nil) + } + errLog, errOut := newErrLogger() + + defer func() { + if r := recover(); r != nil { + t.Fatalf("closeContainerLogOutput panicked on nil Out: %v", r) + } + }() + closeContainerLogOutput(c, errLog) + + if errOut.closed { + t.Fatalf("error logger output should not be closed by the helper") + } +} + +// TestCloseContainerLogOutput_NonNil verifies that a non-nil Out is closed +// exactly once, mirroring the previous unconditional behaviour on the +// happy path. +func TestCloseContainerLogOutput_NonNil(t *testing.T) { + out := &trackingOutput{} + c := &container.C{ + DefaultLogger: &log.Logger{Out: out}, + } + errLog, _ := newErrLogger() + + closeContainerLogOutput(c, errLog) + if !out.closed { + t.Fatalf("expected old container log output to be closed") + } +} + +// TestCloseContainerLogOutput_CloseError verifies that an error returned +// from the underlying Close is reported through errLogger and does not +// propagate as a panic. +func TestCloseContainerLogOutput_CloseError(t *testing.T) { + out := &trackingOutput{closeErr: errors.New("disk full")} + c := &container.C{ + DefaultLogger: &log.Logger{Out: out}, + } + errLog, _ := newErrLogger() + + defer func() { + if r := recover(); r != nil { + t.Fatalf("closeContainerLogOutput panicked: %v", r) + } + }() + closeContainerLogOutput(c, errLog) + if !out.closed { + t.Fatalf("expected Close to be invoked even when it returns an error") + } +} + +// TestCloseContainerLogOutput_NilContainer verifies the helper tolerates a +// nil container / nil DefaultLogger, so callers do not need to add extra +// nil checks of their own. +func TestCloseContainerLogOutput_NilContainer(t *testing.T) { + errLog, _ := newErrLogger() + + defer func() { + if r := recover(); r != nil { + t.Fatalf("closeContainerLogOutput panicked on nil container: %v", r) + } + }() + closeContainerLogOutput(nil, errLog) + + c := &container.C{} // DefaultLogger is nil + closeContainerLogOutput(c, errLog) +} From 58e8a11423e140ad37063ce8dfc446de4c1591ed Mon Sep 17 00:00:00 2001 From: "fox.cpp" Date: Sun, 24 May 2026 01:17:29 +0300 Subject: [PATCH 7/7] maddy 0.9.5 --- .version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.version b/.version index a602fc9e..b0bb8785 100644 --- a/.version +++ b/.version @@ -1 +1 @@ -0.9.4 +0.9.5