From f7df1fd0066ccf1107d6cbe299fde4e8d62b1efc Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Fri, 30 Aug 2024 12:34:05 +0200 Subject: [PATCH 1/2] fix: keep defer, but conditionalize Signed-off-by: Danny Kopping --- coderd/notifications/dispatch/smtp.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/coderd/notifications/dispatch/smtp.go b/coderd/notifications/dispatch/smtp.go index 7bbff3fcb8879..8a0cdbcf882fa 100644 --- a/coderd/notifications/dispatch/smtp.go +++ b/coderd/notifications/dispatch/smtp.go @@ -183,6 +183,15 @@ func (s *SMTPHandler) dispatch(subject, htmlBody, plainBody, to string) Delivery if err != nil { return true, xerrors.Errorf("message transmission: %w", err) } + closed := false + // Close the message when this method exits in order to not leak resources. Even though we're calling this explicitly + // further down, the method may exit before then. + defer func() { + // If we try close an already-closed writer, it'll send a subsequent request to the server which is invalid. + if !closed { + _ = message.Close() + } + }() // Create message headers. msg := &bytes.Buffer{} @@ -250,6 +259,7 @@ func (s *SMTPHandler) dispatch(subject, htmlBody, plainBody, to string) Delivery return false, xerrors.Errorf("write body buffer: %w", err) } + closed = true if err = message.Close(); err != nil { return true, xerrors.Errorf("delivery failure: %w", err) } From 8d8cd98b51f75f4ca66287c1305218eb794ad342 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Fri, 30 Aug 2024 12:48:34 +0200 Subject: [PATCH 2/2] Use sync.OnceValue Signed-off-by: Danny Kopping --- coderd/notifications/dispatch/smtp.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/coderd/notifications/dispatch/smtp.go b/coderd/notifications/dispatch/smtp.go index 8a0cdbcf882fa..107e9e2e8c723 100644 --- a/coderd/notifications/dispatch/smtp.go +++ b/coderd/notifications/dispatch/smtp.go @@ -183,14 +183,14 @@ func (s *SMTPHandler) dispatch(subject, htmlBody, plainBody, to string) Delivery if err != nil { return true, xerrors.Errorf("message transmission: %w", err) } - closed := false + closeOnce := sync.OnceValue(func() error { + return message.Close() + }) // Close the message when this method exits in order to not leak resources. Even though we're calling this explicitly // further down, the method may exit before then. defer func() { // If we try close an already-closed writer, it'll send a subsequent request to the server which is invalid. - if !closed { - _ = message.Close() - } + _ = closeOnce() }() // Create message headers. @@ -259,8 +259,7 @@ func (s *SMTPHandler) dispatch(subject, htmlBody, plainBody, to string) Delivery return false, xerrors.Errorf("write body buffer: %w", err) } - closed = true - if err = message.Close(); err != nil { + if err = closeOnce(); err != nil { return true, xerrors.Errorf("delivery failure: %w", err) }