accounts/keystore: serialize scans with deletion

accounts/keystore/account_cache.go

@@ -73,6 +73,7 @@ type accountCache struct {
 	throttle *time.Timer
 	notify   chan struct{}
 	fileC    fileCache
+	scanMu   sync.Mutex
 }
 
 func newAccountCache(keydir string) (*accountCache, chan struct{}) {
@@ -133,6 +134,17 @@ func (ac *accountCache) delete(removed accounts.Account) {
 	}
 }
 
+func (ac *accountCache) deleteFile(removed accounts.Account) error {
+	ac.scanMu.Lock()
+	defer ac.scanMu.Unlock()
+
+	if err := os.Remove(removed.URL.Path); err != nil {
+		return err
+	}
+	ac.delete(removed)
+	return nil
+}
+
 // deleteByFile removes an account referenced by the given path.
 func (ac *accountCache) deleteByFile(path string) {
 	ac.mu.Lock()
@@ -253,6 +265,9 @@ func (ac *accountCache) close() {
 // scanAccounts checks if any changes have occurred on the filesystem, and
 // updates the account cache accordingly
 func (ac *accountCache) scanAccounts() error {
+	ac.scanMu.Lock()
+	defer ac.scanMu.Unlock()
+
 	// Scan the entire folder metadata for file changes
 	creates, deletes, updates, err := ac.fileC.scan(ac.keydir)
 	if err != nil {

accounts/keystore/keystore.go

@@ -25,7 +25,6 @@ import (
 	crand "crypto/rand"
 	"errors"
 	"math/big"
-	"os"
 	"path/filepath"
 	"reflect"
 	"runtime"
@@ -251,16 +250,11 @@ func (ks *KeyStore) Delete(a accounts.Account, passphrase string) error {
 	if err != nil {
 		return err
 	}
-	// The order is crucial here. The key is dropped from the
-	// cache after the file is gone so that a reload happening in
-	// between won't insert it into the cache again.
-	err = os.Remove(a.URL.Path)
-	if err == nil {
-		ks.cache.delete(a)
-		ks.refreshWallets()
+	if err := ks.cache.deleteFile(a); err != nil {
+		return err
 	}
-
-	return err
+	ks.refreshWallets()
+	return nil
 }
 
 // SignHash calculates a ECDSA signature for the given hash. The produced

accounts/keystore/keystore_test.go

@@ -78,6 +78,36 @@ func TestKeyStore(t *testing.T) {
 	}
 }
 
+type removeKeyOnReadStore struct {
+	keyStore
+}
+
+func (s *removeKeyOnReadStore) GetKey(addr common.Address, filename, auth string) (*Key, error) {
+	key, err := s.keyStore.GetKey(addr, filename, auth)
+	if err != nil {
+		return key, err
+	}
+	if err := os.Remove(filename); err != nil {
+		return nil, err
+	}
+	return key, nil
+}
+
+func TestDeleteFileDisappearsAfterKeyLookup(t *testing.T) {
+	t.Parallel()
+	_, ks := tmpKeyStore(t)
+
+	account, err := ks.NewAccount("")
+	if err != nil {
+		t.Fatal(err)
+	}
+	ks.storage = &removeKeyOnReadStore{keyStore: ks.storage}
+
+	if err := ks.Delete(account, ""); !os.IsNotExist(err) {
+		t.Fatalf("Delete error = %v, want file-not-found", err)
+	}
+}
+
 func TestSign(t *testing.T) {
 	t.Parallel()
 	_, ks := tmpKeyStore(t)