Add osutil.AtomicWriter

This captures the common pattern of writing to a temp file and moving it
to it's real name only if everything went well. It reduces the amount of
code in some places where we do this, but maybe not as much as I
expected because the upgrade thing is still a special snowflake...

cmd/syncthing/gui_csrf.go

@@ -12,7 +12,6 @@ import (
 	"net/http"
 	"os"
 	"strings"
-	"time"
 
 	"github.com/syncthing/syncthing/internal/osutil"
 	"github.com/syncthing/syncthing/internal/sync"
@@ -91,28 +90,20 @@ func newCsrfToken() string {
 }
 
 func saveCsrfTokens() {
-	name := locations[locCsrfTokens]
-	tmp := fmt.Sprintf("%s.tmp.%d", name, time.Now().UnixNano())
+	// We're ignoring errors in here. It's not super critical and there's
+	// nothing relevant we can do about them anyway...
 
-	f, err := os.OpenFile(tmp, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0644)
+	name := locations[locCsrfTokens]
+	f, err := osutil.CreateAtomic(name, 0600)
 	if err != nil {
 		return
 	}
-	defer os.Remove(tmp)
 
 	for _, t := range csrfTokens {
-		_, err := fmt.Fprintln(f, t)
-		if err != nil {
-			return
-		}
+		fmt.Fprintln(f, t)
 	}
 
-	err = f.Close()
-	if err != nil {
-		return
-	}
-
-	osutil.Rename(tmp, name)
+	f.Close()
 }
 
 func loadCsrfTokens() {

internal/config/wrapper.go

@@ -7,9 +7,7 @@
 package config
 
 import (
-	"io/ioutil"
 	"os"
-	"path/filepath"
 
 	"github.com/syncthing/protocol"
 	"github.com/syncthing/syncthing/internal/events"
@@ -283,24 +281,20 @@ func (w *Wrapper) IgnoredDevice(id protocol.DeviceID) bool {
 
 // Save writes the configuration to disk, and generates a ConfigSaved event.
 func (w *Wrapper) Save() error {
-	fd, err := ioutil.TempFile(filepath.Dir(w.path), "cfg")
+	fd, err := osutil.CreateAtomic(w.path, 0600)
 	if err != nil {
 		return err
 	}
-	defer os.Remove(fd.Name())
 
-	err = w.cfg.WriteXML(fd)
-	if err != nil {
+	if err := w.cfg.WriteXML(fd); err != nil {
 		fd.Close()
 		return err
 	}
 
-	err = fd.Close()
-	if err != nil {
+	if err := fd.Close(); err != nil {
 		return err
 	}
 
 	events.Default.Log(events.ConfigSaved, w.cfg)
-
-	return osutil.Rename(fd.Name(), w.path)
+	return nil
 }

internal/model/model.go

@@ -13,7 +13,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"net"
 	"os"
 	"path/filepath"
@@ -936,30 +935,17 @@ func (m *Model) SetIgnores(folder string, content []string) error {
 		return fmt.Errorf("Folder %s does not exist", folder)
 	}
 
-	fd, err := ioutil.TempFile(cfg.Path(), ".syncthing.stignore-"+folder)
+	fd, err := osutil.CreateAtomic(filepath.Join(cfg.Path(), ".stignore"), 0644)
 	if err != nil {
 		l.Warnln("Saving .stignore:", err)
 		return err
 	}
-	defer os.Remove(fd.Name())
 
 	for _, line := range content {
-		_, err = fmt.Fprintln(fd, line)
-		if err != nil {
-			l.Warnln("Saving .stignore:", err)
-			return err
-		}
+		fmt.Fprintln(fd, line)
 	}
 
-	err = fd.Close()
-	if err != nil {
-		l.Warnln("Saving .stignore:", err)
-		return err
-	}
-
-	file := filepath.Join(cfg.Path(), ".stignore")
-	err = osutil.Rename(fd.Name(), file)
-	if err != nil {
+	if err := fd.Close(); err != nil {
 		l.Warnln("Saving .stignore:", err)
 		return err
 	}

internal/osutil/atomic.go

@@ -0,0 +1,101 @@
+// Copyright (C) 2015 The Syncthing Authors.
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at http://mozilla.org/MPL/2.0/.
+
+package osutil
+
+import (
+	"errors"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"runtime"
+)
+
+var (
+	ErrClosed  = errors.New("write to closed writer")
+	TempPrefix = ".syncthing.tmp."
+)
+
+// An AtomicWriter is an *os.File that writes to a temporary file in the same
+// directory as the final path. On successfull Close the file is renamed to
+// it's final path. Any error on Write or during Close is accumulated and
+// returned on Close, so a lazy user can ignore errors until Close.
+type AtomicWriter struct {
+	path string
+	next *os.File
+	err  error
+}
+
+// CreateAtomic is like os.Create with a FileMode, except a temporary file
+// name is used instead of the given name.
+func CreateAtomic(path string, mode os.FileMode) (*AtomicWriter, error) {
+	fd, err := ioutil.TempFile(filepath.Dir(path), TempPrefix)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := os.Chmod(fd.Name(), mode); err != nil {
+		fd.Close()
+		os.Remove(fd.Name())
+		return nil, err
+	}
+
+	w := &AtomicWriter{
+		path: path,
+		next: fd,
+	}
+
+	return w, nil
+}
+
+// Write is like io.Writer, but is a no-op on an already failed AtomicWriter.
+func (w *AtomicWriter) Write(bs []byte) (int, error) {
+	if w.err != nil {
+		return 0, w.err
+	}
+	n, err := w.next.Write(bs)
+	if err != nil {
+		w.err = err
+		w.next.Close()
+	}
+	return n, err
+}
+
+// Close closes the temporary file and renames it to the final path. It is
+// invalid to call Write() or Close() after Close().
+func (w *AtomicWriter) Close() error {
+	if w.err != nil {
+		return w.err
+	}
+
+	// Try to not leave temp file around, but ignore error.
+	defer os.Remove(w.next.Name())
+
+	if err := w.next.Close(); err != nil {
+		w.err = err
+		return err
+	}
+
+	// Remove the destination file, on Windows only. If it fails, and not due
+	// to the file not existing, we won't be able to complete the rename
+	// either. Return this error because it may be more informative. On non-
+	// Windows we want the atomic rename behavior so we don't attempt remove.
+	if runtime.GOOS == "windows" {
+		if err := os.Remove(w.path); err != nil && !os.IsNotExist(err) {
+			return err
+		}
+	}
+
+	if err := os.Rename(w.next.Name(), w.path); err != nil {
+		w.err = err
+		return err
+	}
+
+	// Set w.err to return appropriately for any future operations.
+	w.err = ErrClosed
+
+	return nil
+}

internal/osutil/atomic_test.go

@@ -0,0 +1,85 @@
+// Copyright (C) 2015 The Syncthing Authors.
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at http://mozilla.org/MPL/2.0/.
+
+package osutil
+
+import (
+	"bytes"
+	"io/ioutil"
+	"os"
+	"testing"
+)
+
+func TestCreateAtomicCreate(t *testing.T) {
+	os.RemoveAll("testdata")
+	defer os.RemoveAll("testdata")
+
+	if err := os.Mkdir("testdata", 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	w, err := CreateAtomic("testdata/file", 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	n, err := w.Write([]byte("hello"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if n != 5 {
+		t.Fatal("written bytes", n, "!= 5")
+	}
+
+	if _, err := ioutil.ReadFile("testdata/file"); err == nil {
+		t.Fatal("file should not exist")
+	}
+
+	if err := w.Close(); err != nil {
+		t.Fatal(err)
+	}
+
+	bs, err := ioutil.ReadFile("testdata/file")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !bytes.Equal(bs, []byte("hello")) {
+		t.Error("incorrect data")
+	}
+}
+
+func TestCreateAtomicReplace(t *testing.T) {
+	os.RemoveAll("testdata")
+	defer os.RemoveAll("testdata")
+
+	if err := os.Mkdir("testdata", 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := ioutil.WriteFile("testdata/file", []byte("some old data"), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	w, err := CreateAtomic("testdata/file", 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := w.Write([]byte("hello")); err != nil {
+		t.Fatal(err)
+	}
+	if err := w.Close(); err != nil {
+		t.Fatal(err)
+	}
+
+	bs, err := ioutil.ReadFile("testdata/file")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !bytes.Equal(bs, []byte("hello")) {
+		t.Error("incorrect data")
+	}
+}