Always use transaction in JSONFile.Save()

go/engine/device_register.go

@@ -66,9 +66,6 @@ func (d *DeviceRegister) Run(ctx *Context) error {
 		if err := wr.SetDeviceID(d.deviceID); err != nil {
 			return err
 		}
-		if err := wr.Save(); err != nil {
-			return err
-		}
 		ctx.LogUI.Debug("Setting Device ID to %s", d.deviceID)
 	}
 

go/libkb/account.go

@@ -6,9 +6,10 @@ package libkb
 import (
 	"errors"
 	"fmt"
+	"time"
+
 	keybase1 "github.com/keybase/client/go/protocol"
 	triplesec "github.com/keybase/go-triplesec"
-	"time"
 )
 
 type timedGenericKey struct {
@@ -383,34 +384,27 @@ func (a *Account) UserInfo() (uid keybase1.UID, username NormalizedUsername,
 // SaveState saves the logins state to memory, and to the user
 // config file.
 func (a *Account) SaveState(sessionID, csrf string, username NormalizedUsername, uid keybase1.UID, deviceID keybase1.DeviceID) error {
-	cw, err := a.newUserConfigWriter(username, uid, deviceID)
-	if err != nil {
-		return err
-	}
-	if err := cw.Save(); err != nil {
+	if err := a.saveUserConfig(username, uid, deviceID); err != nil {
 		return err
 	}
 	return a.LocalSession().SetLoggedIn(sessionID, csrf, username, uid, deviceID)
 }
 
-func (a *Account) newUserConfigWriter(username NormalizedUsername, uid keybase1.UID, deviceID keybase1.DeviceID) (ConfigWriter, error) {
+func (a *Account) saveUserConfig(username NormalizedUsername, uid keybase1.UID, deviceID keybase1.DeviceID) error {
 	cw := a.G().Env.GetConfigWriter()
 	if cw == nil {
-		return nil, NoConfigWriterError{}
+		return NoConfigWriterError{}
 	}
 
 	if err := a.LoginSession().Clear(); err != nil {
-		return nil, err
+		return err
 	}
 	salt, err := a.LoginSession().Salt()
 	if err != nil {
-		return nil, err
-	}
-	if err := cw.SetUserConfig(NewUserConfig(uid, username, salt, deviceID), false); err != nil {
-		return nil, err
+		return err
 	}
 
-	return cw, nil
+	return cw.SetUserConfig(NewUserConfig(uid, username, salt, deviceID), false)
 }
 
 func (a *Account) Dump() {

go/libkb/interfaces.go

@@ -166,7 +166,6 @@ type ConfigWriter interface {
 	SetUpdatePreferenceSnoozeUntil(keybase1.Time) error
 	SetUpdateLastChecked(keybase1.Time) error
 	Reset()
-	Save() error
 	BeginTransaction() (ConfigWriterTransacter, error)
 }
 

go/libkb/json.go

@@ -5,11 +5,13 @@ package libkb
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
-	jsonw "github.com/keybase/go-jsonw"
 	"io"
 	"os"
 	"sync"
+
+	jsonw "github.com/keybase/go-jsonw"
 )
 
 type jsonFileTransaction struct {
@@ -84,14 +86,6 @@ func (f *JSONFile) Nuke() error {
 	return err
 }
 
-func (f *JSONFile) getFilename() string {
-	tx := f.getTx()
-	if tx != nil {
-		return tx.tmpname
-	}
-	return f.filename
-}
-
 func (f *JSONFile) BeginTransaction() (ConfigWriterTransacter, error) {
 	tx, err := newJSONFileTransaction(f)
 	if err != nil {
@@ -114,10 +108,25 @@ func (f *JSONFile) setTx(tx *jsonFileTransaction) error {
 	return nil
 }
 
-func (f *JSONFile) getTx() *jsonFileTransaction {
+func (f *JSONFile) getOrMakeTx() (*jsonFileTransaction, bool, error) {
 	f.txMutex.Lock()
 	defer f.txMutex.Unlock()
-	return f.tx
+
+	// if a transaction exists, use it
+	if f.tx != nil {
+		return f.tx, false, nil
+	}
+
+	// make a new transaction
+	tx, err := newJSONFileTransaction(f)
+	if err != nil {
+		return nil, false, err
+	}
+
+	f.tx = tx
+
+	// return true so caller knows that a transaction was created
+	return f.tx, true, nil
 }
 
 func newJSONFileTransaction(f *JSONFile) (*jsonFileTransaction, error) {
@@ -131,13 +140,43 @@ func newJSONFileTransaction(f *JSONFile) (*jsonFileTransaction, error) {
 }
 
 func (f *JSONFile) Save() error {
-	if err := f.save(f.getFilename(), true, 0); err != nil {
+	tx, txCreated, err := f.getOrMakeTx()
+	if err != nil {
 		return err
 	}
+	if txCreated {
+		// if Save() created a transaction, then abort it if it
+		// still exists on exit
+		defer func() {
+			if tx != nil {
+				tx.Abort()
+			}
+		}()
+	}
+
+	if err := f.save(); err != nil {
+		return err
+	}
+
+	if txCreated {
+		// this Save() call created a transaction, so commit it
+		if err := tx.Commit(); err != nil {
+			return err
+		}
+
+		// Commit worked, clear the transaction so defer() doesn't
+		// abort it.
+		tx = nil
+	}
+
 	return nil
 }
 
-func (f *JSONFile) save(filename string, pretty bool, mode os.FileMode) (err error) {
+func (f *JSONFile) save() (err error) {
+	if f.tx == nil {
+		return errors.New("save() called with nil transaction")
+	}
+	filename := f.tx.tmpname
 	f.G().Log.Debug("+ saving %s file %s", f.which, filename)
 
 	err = MakeParentDirs(filename)
@@ -162,25 +201,17 @@ func (f *JSONFile) save(filename string, pretty bool, mode os.FileMode) (err err
 	}
 	var writer *os.File
 	flags := (os.O_WRONLY | os.O_CREATE | os.O_TRUNC)
-	if mode == 0 {
-		mode = PermFile // By default, secrecy
-	}
-	writer, err = os.OpenFile(filename, flags, mode)
+	writer, err = os.OpenFile(filename, flags, PermFile)
 	if err != nil {
 		f.G().Log.Errorf("Failed to open %s file %s for writing: %s",
 			f.which, filename, err)
 		return err
 	}
 	defer writer.Close()
 
-	if pretty {
-		encoded, err := json.MarshalIndent(dat, "", "    ")
-		if err == nil {
-			_, err = writer.Write(encoded)
-		}
-	} else {
-		encoder := json.NewEncoder(writer)
-		err = encoder.Encode(dat)
+	encoded, err := json.MarshalIndent(dat, "", "    ")
+	if err == nil {
+		_, err = writer.Write(encoded)
 	}
 
 	if err != nil {