Merge pull request zyedidia#3822 from dmaluka/fix-spurious-backups

Fix spurious backups of unmodified files

Author: dmaluka

cmd/micro/micro.go

@@ -352,9 +352,11 @@ func main() {
 			} else {
 				fmt.Println("Micro encountered an error:", errors.Wrap(err, 2).ErrorStack(), "\nIf you can reproduce this error, please report it at https://github.com/zyedidia/micro/issues")
 			}
-			// backup all open buffers
+			// immediately backup all buffers with unsaved changes
 			for _, b := range buffer.OpenBuffers {
-				b.Backup()
+				if b.Modified() {
+					b.Backup()
+				}
 			}
 			exit(1)
 		}
@@ -489,8 +491,6 @@ func DoEvent() {
 		}
 	case f := <-timerChan:
 		f()
-	case b := <-buffer.BackupCompleteChan:
-		b.RequestedBackup = false
 	case <-sighup:
 		exit(0)
 	case <-util.Sigterm:

cmd/micro/micro_test.go

@@ -55,9 +55,11 @@ func startup(args []string) (tcell.SimulationScreen, error) {
 		if err := recover(); err != nil {
 			screen.Screen.Fini()
 			fmt.Println("Micro encountered an error:", err)
-			// backup all open buffers
+			// immediately backup all buffers with unsaved changes
 			for _, b := range buffer.OpenBuffers {
-				b.Backup()
+				if b.Modified() {
+					b.Backup()
+				}
 			}
 			// Print the stack trace too
 			log.Fatalf(errors.Wrap(err, 2).ErrorStack())

internal/buffer/backup.go

@@ -34,75 +34,105 @@ Options: [r]ecover, [i]gnore, [a]bort: `
 
 const backupSeconds = 8
 
-var BackupCompleteChan chan *Buffer
+type backupRequestType int
+
+const (
+	backupCreate = iota
+	backupRemove
+)
+
+type backupRequest struct {
+	buf     *SharedBuffer
+	reqType backupRequestType
+}
+
+var requestedBackups map[*SharedBuffer]bool
 
 func init() {
-	BackupCompleteChan = make(chan *Buffer, 10)
+	requestedBackups = make(map[*SharedBuffer]bool)
+}
+
+func (b *SharedBuffer) RequestBackup() {
+	backupRequestChan <- backupRequest{buf: b, reqType: backupCreate}
 }
 
-func (b *Buffer) RequestBackup() {
-	if !b.RequestedBackup {
-		select {
-		case backupRequestChan <- b:
-		default:
-			// channel is full
+func (b *SharedBuffer) CancelBackup() {
+	backupRequestChan <- backupRequest{buf: b, reqType: backupRemove}
+}
+
+func handleBackupRequest(br backupRequest) {
+	switch br.reqType {
+	case backupCreate:
+		// schedule periodic backup
+		requestedBackups[br.buf] = true
+	case backupRemove:
+		br.buf.RemoveBackup()
+		delete(requestedBackups, br.buf)
+	}
+}
+
+func periodicBackup() {
+	for buf := range requestedBackups {
+		err := buf.Backup()
+		if err == nil {
+			delete(requestedBackups, buf)
 		}
-		b.RequestedBackup = true
 	}
 }
 
-func (b *Buffer) backupDir() string {
+func (b *SharedBuffer) backupDir() string {
 	backupdir, err := util.ReplaceHome(b.Settings["backupdir"].(string))
 	if backupdir == "" || err != nil {
 		backupdir = filepath.Join(config.ConfigDir, "backups")
 	}
 	return backupdir
 }
 
-func (b *Buffer) keepBackup() bool {
+func (b *SharedBuffer) keepBackup() bool {
 	return b.forceKeepBackup || b.Settings["permbackup"].(bool)
 }
 
-// Backup saves the current buffer to the backups directory
-func (b *Buffer) Backup() error {
-	if !b.Settings["backup"].(bool) || b.Path == "" || b.Type != BTDefault {
-		return nil
-	}
-
+func (b *SharedBuffer) writeBackup(path string) (string, error) {
 	backupdir := b.backupDir()
-	if _, err := os.Stat(backupdir); errors.Is(err, fs.ErrNotExist) {
-		os.Mkdir(backupdir, os.ModePerm)
-	}
-
-	name := util.DetermineEscapePath(backupdir, b.AbsPath)
-	if _, err := os.Stat(name); errors.Is(err, fs.ErrNotExist) {
-		_, err = b.overwriteFile(name)
-		if err == nil {
-			BackupCompleteChan <- b
+	if _, err := os.Stat(backupdir); err != nil {
+		if !errors.Is(err, fs.ErrNotExist) {
+			return "", err
+		}
+		if err = os.Mkdir(backupdir, os.ModePerm); err != nil {
+			return "", err
 		}
-		return err
 	}
 
+	name := util.DetermineEscapePath(backupdir, path)
 	tmp := util.AppendBackupSuffix(name)
+
 	_, err := b.overwriteFile(tmp)
 	if err != nil {
 		os.Remove(tmp)
-		return err
+		return name, err
 	}
 	err = os.Rename(tmp, name)
 	if err != nil {
 		os.Remove(tmp)
-		return err
+		return name, err
 	}
 
-	BackupCompleteChan <- b
+	return name, nil
+}
+
+// Backup saves the buffer to the backups directory
+func (b *SharedBuffer) Backup() error {
+	if !b.Settings["backup"].(bool) || b.Path == "" || b.Type != BTDefault {
+		return nil
+	}
 
+	_, err := b.writeBackup(b.AbsPath)
 	return err
 }
 
 // RemoveBackup removes any backup file associated with this buffer
-func (b *Buffer) RemoveBackup() {
-	if !b.Settings["backup"].(bool) || b.keepBackup() || b.Path == "" || b.Type != BTDefault {
+func (b *SharedBuffer) RemoveBackup() {
+	if b.keepBackup() || b.Path == "" || b.Type != BTDefault {
 		return
 	}
 	f := util.DetermineEscapePath(b.backupDir(), b.AbsPath)
@@ -111,7 +141,7 @@ func (b *Buffer) RemoveBackup() {
 
 // ApplyBackup applies the corresponding backup file to this buffer (if one exists)
 // Returns true if a backup was applied
-func (b *Buffer) ApplyBackup(fsize int64) (bool, bool) {
+func (b *SharedBuffer) ApplyBackup(fsize int64) (bool, bool) {
 	if b.Settings["backup"].(bool) && !b.Settings["permbackup"].(bool) && len(b.Path) > 0 && b.Type == BTDefault {
 		backupfile := util.DetermineEscapePath(b.backupDir(), b.AbsPath)
 		if info, err := os.Stat(backupfile); err == nil {
@@ -125,7 +155,7 @@ func (b *Buffer) ApplyBackup(fsize int64) (bool, bool) {
 				if choice%3 == 0 {
 					// recover
 					b.LineArray = NewLineArray(uint64(fsize), FFAuto, backup)
-					b.isModified = true
+					b.setModified()
 					return true, true
 				} else if choice%3 == 1 {
 					// delete

internal/buffer/buffer.go

@@ -13,7 +13,6 @@ import (
 	"strconv"
 	"strings"
 	"sync"
-	"sync/atomic"
 	"time"
 
 	luar "layeh.com/gopher-luar"
@@ -101,7 +100,6 @@ type SharedBuffer struct {
 	diffLock          sync.RWMutex
 	diff              map[int]DiffStatus
 
-	RequestedBackup bool
 	forceKeepBackup bool
 
 	// ReloadDisabled allows the user to disable reloads if they
@@ -126,20 +124,62 @@ type SharedBuffer struct {
 }
 
 func (b *SharedBuffer) insert(pos Loc, value []byte) {
-	b.isModified = true
 	b.HasSuggestions = false
 	b.LineArray.insert(pos, value)
+	b.setModified()
 
 	inslines := bytes.Count(value, []byte{'\n'})
 	b.MarkModified(pos.Y, pos.Y+inslines)
 }
+
 func (b *SharedBuffer) remove(start, end Loc) []byte {
-	b.isModified = true
 	b.HasSuggestions = false
+	defer b.setModified()
 	defer b.MarkModified(start.Y, end.Y)
 	return b.LineArray.remove(start, end)
 }
 
+func (b *SharedBuffer) setModified() {
+	if b.Type.Scratch {
+		return
+	}
+
+	if b.Settings["fastdirty"].(bool) {
+		b.isModified = true
+	} else {
+		var buff [md5.Size]byte
+
+		b.calcHash(&buff)
+		b.isModified = buff != b.origHash
+	}
+
+	if b.isModified {
+		b.RequestBackup()
+	} else {
+		b.CancelBackup()
+	}
+}
+
+// calcHash calculates md5 hash of all lines in the buffer
+func (b *SharedBuffer) calcHash(out *[md5.Size]byte) {
+	h := md5.New()
+
+	if len(b.lines) > 0 {
+		h.Write(b.lines[0].data)
+
+		for _, l := range b.lines[1:] {
+			if b.Endings == FFDos {
+				h.Write([]byte{'\r', '\n'})
+			} else {
+				h.Write([]byte{'\n'})
+			}
+			h.Write(l.data)
+		}
+	}
+
+	h.Sum((*out)[:0])
+}
+
 // MarkModified marks the buffer as modified for this frame
 // and performs rehighlighting if syntax highlighting is enabled
 func (b *SharedBuffer) MarkModified(start, end int) {
@@ -187,7 +227,6 @@ type Buffer struct {
 	*EventHandler
 	*SharedBuffer
 
-	fini        int32
 	cursors     []*Cursor
 	curCursor   int
 	StartCursor Loc
@@ -416,7 +455,7 @@ func NewBuffer(r io.Reader, size int64, path string, startcursor Loc, btype BufT
 		} else if !hasBackup {
 			// since applying a backup does not save the applied backup to disk, we should
 			// not calculate the original hash based on the backup data
-			calcHash(b, &b.origHash)
+			b.calcHash(&b.origHash)
 		}
 	}
 
@@ -458,13 +497,11 @@ func (b *Buffer) Fini() {
 	if !b.Modified() {
 		b.Serialize()
 	}
-	b.RemoveBackup()
+	b.CancelBackup()
 
 	if b.Type == BTStdout {
 		fmt.Fprint(util.Stdout, string(b.Bytes()))
 	}
-
-	atomic.StoreInt32(&(b.fini), int32(1))
 }
 
 // GetName returns the name that should be displayed in the statusline
@@ -494,8 +531,6 @@ func (b *Buffer) Insert(start Loc, text string) {
 		b.EventHandler.cursors = b.cursors
 		b.EventHandler.active = b.curCursor
 		b.EventHandler.Insert(start, text)
-
-		b.RequestBackup()
 	}
 }
 
@@ -505,8 +540,6 @@ func (b *Buffer) Remove(start, end Loc) {
 		b.EventHandler.cursors = b.cursors
 		b.EventHandler.active = b.curCursor
 		b.EventHandler.Remove(start, end)
-
-		b.RequestBackup()
 	}
 }
 
@@ -558,7 +591,7 @@ func (b *Buffer) ReOpen() error {
 		if len(data) > LargeFileThreshold {
 			b.Settings["fastdirty"] = true
 		} else {
-			calcHash(b, &b.origHash)
+			b.calcHash(&b.origHash)
 		}
 	}
 	b.isModified = false
@@ -633,18 +666,7 @@ func (b *Buffer) Shared() bool {
 // Modified returns if this buffer has been modified since
 // being opened
 func (b *Buffer) Modified() bool {
-	if b.Type.Scratch {
-		return false
-	}
-
-	if b.Settings["fastdirty"].(bool) {
-		return b.isModified
-	}
-
-	var buff [md5.Size]byte
-
-	calcHash(b, &buff)
-	return buff != b.origHash
+	return b.isModified
 }
 
 // Size returns the number of bytes in the current buffer
@@ -663,26 +685,6 @@ func (b *Buffer) Size() int {
 	return nb
 }
 
-// calcHash calculates md5 hash of all lines in the buffer
-func calcHash(b *Buffer, out *[md5.Size]byte) {
-	h := md5.New()
-
-	if len(b.lines) > 0 {
-		h.Write(b.lines[0].data)
-
-		for _, l := range b.lines[1:] {
-			if b.Endings == FFDos {
-				h.Write([]byte{'\r', '\n'})
-			} else {
-				h.Write([]byte{'\n'})
-			}
-			h.Write(l.data)
-		}
-	}
-
-	h.Sum((*out)[:0])
-}
-
 func parseDefFromFile(f config.RuntimeFile, header *highlight.Header) *highlight.Def {
 	data, err := f.Data()
 	if err != nil {
@@ -1233,7 +1235,6 @@ func (b *Buffer) FindMatchingBrace(start Loc) (Loc, bool, bool) {
 func (b *Buffer) Retab() {
 	toSpaces := b.Settings["tabstospaces"].(bool)
 	tabsize := util.IntOpt(b.Settings["tabsize"])
-	dirty := false
 
 	for i := 0; i < b.LinesNum(); i++ {
 		l := b.LineBytes(i)
@@ -1254,10 +1255,9 @@ func (b *Buffer) Retab() {
 		b.Unlock()
 
 		b.MarkModified(i, i)
-		dirty = true
 	}
 
-	b.isModified = dirty
+	b.setModified()
 }
 
 // ParseCursorLocation turns a cursor location like 10:5 (LINE:COL)