Tim Culverhouse: 1 email: don't keep files open after downloading 5 files changed, 38 insertions(+), 32 deletions(-)
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~rockorager/offmap/patches/37247/mbox | git am -3Learn more about email & git
Store the absolute filename to the downloaded email instead of keeping an open file descriptor. Use os.Rename when creating maildir emails to reduce I/O. Fix tests. Reported-by: inwit <inwit@sindominio.net> Signed-off-by: Tim Culverhouse <tim@timculverhouse.com> --- v2: read the entire file into a buffer for reading email.go | 28 +++++++++++++++++++--------- imap/imap.go | 8 +------- maildir/maildir.go | 9 ++------- maildir/update.go | 10 ++++++---- maildir/update_test.go | 15 ++++++++++----- 5 files changed, 38 insertions(+), 32 deletions(-) diff --git a/email.go b/email.go index c11fd82bca8c..7d9fdf50627d 100644 --- a/email.go +++ b/email.go @@ -1,8 +1,10 @@ package offmap import ( + "bytes" "fmt" "io" + "os" "time" "github.com/emersion/go-maildir" @@ -23,8 +25,9 @@ type Email struct { // Date is the best known time for the email creation Date time.Time `json:"-"` - // Body is the RFC822 body of the email (ie, the entire email) - body io.ReadCloser + // filename is the absolute path of the email on disk + filename string + body io.Reader // size is the size of email in bytes size int } @@ -40,17 +43,28 @@ func NewEmail(mbox *Mailbox, uid uint32, key string, flags []maildir.Flag) *Emai } } -func (e *Email) SetBody(body io.ReadCloser) { - e.body = body +func (e *Email) SetFilename(path string) { + e.filename = path } func (e *Email) Read(p []byte) (int, error) { - if e.body == nil { + if e.filename == "" { return 0, fmt.Errorf("email: no body") } + if e.body == nil { + b, err := os.ReadFile(e.filename) + if err != nil { + return 0, fmt.Errorf("email: %v", err) + } + e.body = bytes.NewBuffer(b) + } return e.body.Read(p) } +func (e *Email) Filename() string { + return e.filename +} + func (e *Email) SetSize(i int) { e.size = i } @@ -58,7 +72,3 @@ func (e *Email) SetSize(i int) { func (e *Email) Len() int { return e.size } - -func (e *Email) Close() error { - return e.body.Close() -} diff --git a/imap/imap.go b/imap/imap.go index 74e3eb55e032..4bf9e918a517 100644 --- a/imap/imap.go +++ b/imap/imap.go @@ -220,12 +220,6 @@ func (s *Store) DownloadEmail(emls []*offmap.Email) []*offmap.Email { atomic.AddInt32(&dled, 1) cur := atomic.LoadInt32(&dled) f.Close() - // Reopen but with read only mode - f, err = os.Open(filepath) - if err != nil { - log.Errorf("imap: %v", err) - continue - } emlMu.Lock() eml, ok := emlMap[filename] emlMu.Unlock() @@ -233,7 +227,7 @@ func (s *Store) DownloadEmail(emls []*offmap.Email) []*offmap.Email { log.Errorf("imap: %s: eml not found: %s", status.Name, filename) continue } - eml.SetBody(f) + eml.SetFilename(filepath) log.Tracef("imap: (client %d) downloaded %d of %d", client.id, cur, total) } msgWg.Done() diff --git a/maildir/maildir.go b/maildir/maildir.go index 2682066e3fa4..5209332d285d 100644 --- a/maildir/maildir.go +++ b/maildir/maildir.go @@ -66,17 +66,12 @@ func (s *Store) DownloadEmail(emls []*offmap.Email) []*offmap.Email { log.Errorf("maildir: couldn't find email %d", eml.UID) continue } - f, err := os.Open(filename) + fi, err := os.Stat(filename) if err != nil { log.Errorf("maildir: couldn't open file %d", eml.UID) continue } - fi, err := f.Stat() - if err != nil { - log.Errorf("maildir: couldn't open file %d", eml.UID) - continue - } - eml.SetBody(f) + eml.SetFilename(filename) eml.Date = fi.ModTime() eml.SetSize(int(fi.Size())) } diff --git a/maildir/update.go b/maildir/update.go index 2e8a1c56a59c..dc0d6ac6e53b 100644 --- a/maildir/update.go +++ b/maildir/update.go @@ -2,7 +2,6 @@ package maildir import ( "fmt" - "io" "os" "path" @@ -109,13 +108,16 @@ func (s *Store) renameMaildir(orig string, dest string) error { func (s *Store) createEmail(mbox string, emls []*offmap.Email) error { dir := maildir.Dir(path.Join(s.root, mbox)) for _, eml := range emls { - defer eml.Close() key, wc, err := dir.Create(eml.Flags) if err != nil { return fmt.Errorf("maildir: could not create email: %v", err) } - defer wc.Close() - _, err = io.Copy(wc, eml) + wc.Close() + dest, err := dir.Filename(key) + if err != nil { + return fmt.Errorf("maildir: could not write email: %v", err) + } + err = os.Rename(eml.Filename(), dest) if err != nil { return fmt.Errorf("maildir: could not write email: %v", err) } diff --git a/maildir/update_test.go b/maildir/update_test.go index fbbe2f23c742..6faa060d47fb 100644 --- a/maildir/update_test.go +++ b/maildir/update_test.go @@ -1,8 +1,8 @@ package maildir import ( - "bytes" - "io" + "os" + "path" "testing" "git.sr.ht/~rockorager/offmap" @@ -75,9 +75,14 @@ func TestCreateEmail(t *testing.T) { mailbox := mboxes["Inbox"] eml := offmap.NewEmail(mailbox, 0, "key", []maildir.Flag{'S'}) - buf := bytes.NewBufferString("Email body") - rc := io.NopCloser(buf) - eml.SetBody(rc) + tmpEml := path.Join(t.TempDir(), "tmpEmail") + f, err := os.Create(tmpEml) + if err != nil { + t.Fatal(err) + } + f.WriteString("email body") + f.Close() + eml.SetFilename(tmpEml) mboxes, err = store.readCurrentState() assert.NoError(t, err) -- 2.38.1
builds.sr.ht <builds@sr.ht>offmap/patches/.build.yml: SUCCESS in 1m15s [email: don't keep files open after downloading][0] v2 from [Tim Culverhouse][1] [0]: https://lists.sr.ht/~rockorager/offmap/patches/37247 [1]: mailto:tim@timculverhouse.com ✓ #895050 SUCCESS offmap/patches/.build.yml https://builds.sr.ht/~rockorager/job/895050
Reviewed-by: Moritz Poldrack <moritz@poldrack.dev