From 904ffacb0e521218ba1f41e2e5c26d9ac41c9969 Mon Sep 17 00:00:00 2001 From: Nguyễn Gia Phong Date: Thu, 20 Jan 2022 01:10:08 +0700 Subject: maildir,notmuch: avoid leaking open files Previously, Message.NewReader returned the wrapped buffered reader without a reference to the opened file, so the files descriptors were left unclosed after reading. Now, the file reader is returned directly and closed on the call site. Buffering is not needed here because it is an implementation detail of go-message. Fixes: https://todo.sr.ht/~rjarry/aerc/9 --- worker/lib/parse.go | 3 ++- worker/lib/parse_test.go | 18 ++++++------------ worker/maildir/message.go | 9 ++------- worker/maildir/search.go | 1 + worker/maildir/worker.go | 1 + worker/notmuch/message.go | 9 ++------- worker/notmuch/worker.go | 1 + 7 files changed, 15 insertions(+), 27 deletions(-) diff --git a/worker/lib/parse.go b/worker/lib/parse.go index 87f94bdd..6d73a1f5 100644 --- a/worker/lib/parse.go +++ b/worker/lib/parse.go @@ -211,7 +211,7 @@ func parseAddressList(h *mail.Header, key string) ([]*mail.Address, error) { // RawMessage is an interface that describes a raw message type RawMessage interface { - NewReader() (io.Reader, error) + NewReader() (io.ReadCloser, error) ModelFlags() ([]models.Flag, error) Labels() ([]string, error) UID() uint32 @@ -225,6 +225,7 @@ func MessageInfo(raw RawMessage) (*models.MessageInfo, error) { if err != nil { return nil, err } + defer r.Close() msg, err := message.Read(r) if err != nil { return nil, fmt.Errorf("could not read message: %v", err) diff --git a/worker/lib/parse_test.go b/worker/lib/parse_test.go index 5c0a1b17..12190981 100644 --- a/worker/lib/parse_test.go +++ b/worker/lib/parse_test.go @@ -1,9 +1,9 @@ package lib import ( - "bytes" "io" "io/ioutil" + "os" "path/filepath" "testing" @@ -36,23 +36,17 @@ func TestMessageInfoHandledError(t *testing.T) { } type mockRawMessage struct { - body []byte + path string } -func newMockRawMessage(body []byte) *mockRawMessage { +func newMockRawMessageFromPath(p string) *mockRawMessage { return &mockRawMessage{ - body: body, + path: p, } } -func newMockRawMessageFromPath(p string) *mockRawMessage { - b, err := ioutil.ReadFile(p) - die(err) - return newMockRawMessage(b) -} - -func (m *mockRawMessage) NewReader() (io.Reader, error) { - return bytes.NewReader(m.body), nil +func (m *mockRawMessage) NewReader() (io.ReadCloser, error) { + return os.Open(m.path) } func (m *mockRawMessage) ModelFlags() ([]models.Flag, error) { return nil, nil } func (m *mockRawMessage) Labels() ([]string, error) { return nil, nil } diff --git a/worker/maildir/message.go b/worker/maildir/message.go index dbc9ade1..b73e0832 100644 --- a/worker/maildir/message.go +++ b/worker/maildir/message.go @@ -1,7 +1,6 @@ package maildir import ( - "bufio" "fmt" "io" @@ -20,12 +19,8 @@ type Message struct { } // NewReader reads a message into memory and returns an io.Reader for it. -func (m Message) NewReader() (io.Reader, error) { - f, err := m.dir.Open(m.key) - if err != nil { - return nil, err - } - return bufio.NewReader(f), nil +func (m Message) NewReader() (io.ReadCloser, error) { + return m.dir.Open(m.key) } // Flags fetches the set of flags currently applied to the message. diff --git a/worker/maildir/search.go b/worker/maildir/search.go index edfb8d84..7f97fb1a 100644 --- a/worker/maildir/search.go +++ b/worker/maildir/search.go @@ -159,6 +159,7 @@ func (w *Worker) searchKey(key uint32, criteria *searchCriteria, if err != nil { return false, err } + defer reader.Close() bytes, err := ioutil.ReadAll(reader) if err != nil { return false, err diff --git a/worker/maildir/worker.go b/worker/maildir/worker.go index 621b19e3..a671d730 100644 --- a/worker/maildir/worker.go +++ b/worker/maildir/worker.go @@ -432,6 +432,7 @@ func (w *Worker) handleFetchFullMessages(msg *types.FetchFullMessages) error { w.worker.Logger.Printf("could not get message reader: %v", err) return err } + defer r.Close() w.worker.PostMessage(&types.FullMessage{ Message: types.RespondTo(msg), Content: &models.FullMessage{ diff --git a/worker/notmuch/message.go b/worker/notmuch/message.go index c95c9bac..e39fb17a 100644 --- a/worker/notmuch/message.go +++ b/worker/notmuch/message.go @@ -4,7 +4,6 @@ package notmuch import ( - "bufio" "fmt" "io" "os" @@ -23,16 +22,12 @@ type Message struct { } // NewReader returns a reader for a message -func (m *Message) NewReader() (io.Reader, error) { +func (m *Message) NewReader() (io.ReadCloser, error) { name, err := m.Filename() if err != nil { return nil, err } - f, err := os.Open(name) - if err != nil { - return nil, err - } - return bufio.NewReader(f), nil + return os.Open(name) } // MessageInfo populates a models.MessageInfo struct for the message. diff --git a/worker/notmuch/worker.go b/worker/notmuch/worker.go index 575cd565..d95d6ba3 100644 --- a/worker/notmuch/worker.go +++ b/worker/notmuch/worker.go @@ -367,6 +367,7 @@ func (w *worker) handleFetchFullMessages(msg *types.FetchFullMessages) error { w.w.Logger.Printf("could not get message reader: %v", err) return err } + defer r.Close() w.w.PostMessage(&types.FullMessage{ Message: types.RespondTo(msg), Content: &models.FullMessage{ -- cgit