node: fix instance dir locking and improve error message

The lock file was ineffective because opening leveldb storage in
read-only mode doesn't really take the lock. Fix it by including a
dedicated flock library (which is actually split out of goleveldb).

2 files changed, 30 insertions, 24 deletions

diff --git a/node/errors.go b/node/errors.go
index bd5ddeb5d..2e0dadc4d 100644
--- a/node/errors.go
+++ b/node/errors.go
@@ -17,10 +17,28 @@
 package node
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
+	"syscall"
 )
 
+var (
+	ErrDatadirUsed    = errors.New("datadir already used by another process")
+	ErrNodeStopped    = errors.New("node not started")
+	ErrNodeRunning    = errors.New("node already running")
+	ErrServiceUnknown = errors.New("unknown service")
+
+	datadirInUseErrnos = map[uint]bool{11: true, 32: true, 35: true}
+)
+
+func convertFileLockError(err error) error {
+	if errno, ok := err.(syscall.Errno); ok && datadirInUseErrnos[uint(errno)] {
+		return ErrDatadirUsed
+	}
+	return err
+}
+
 // DuplicateServiceError is returned during Node startup if a registered service
 // constructor returns a service of the same type that was already started.
 type DuplicateServiceError struct {
diff --git a/node/node.go b/node/node.go
index e3f0232b4..86cfb29ba 100644
--- a/node/node.go
+++ b/node/node.go
@@ -25,7 +25,6 @@ import (
 	"reflect"
 	"strings"
 	"sync"
-	"syscall"
 
 	"github.com/ethereum/go-ethereum/accounts"
 	"github.com/ethereum/go-ethereum/ethdb"
@@ -34,16 +33,7 @@ import (
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/ethereum/go-ethereum/p2p"
 	"github.com/ethereum/go-ethereum/rpc"
-	"github.com/syndtr/goleveldb/leveldb/storage"
-)
-
-var (
-	ErrDatadirUsed    = errors.New("datadir already used")
-	ErrNodeStopped    = errors.New("node not started")
-	ErrNodeRunning    = errors.New("node already running")
-	ErrServiceUnknown = errors.New("unknown service")
-
-	datadirInUseErrnos = map[uint]bool{11: true, 32: true, 35: true}
+	"github.com/prometheus/prometheus/util/flock"
 )
 
 // Node is a container on which services can be registered.
@@ -52,8 +42,8 @@ type Node struct {
 	config   *Config
 	accman   *accounts.Manager
 
-	ephemeralKeystore string          // if non-empty, the key directory that will be removed by Stop
-	instanceDirLock   storage.Storage // prevents concurrent use of instance directory
+	ephemeralKeystore string         // if non-empty, the key directory that will be removed by Stop
+	instanceDirLock   flock.Releaser // prevents concurrent use of instance directory
 
 	serverConfig p2p.Config
 	server       *p2p.Server // Currently running P2P networking layer
@@ -197,10 +187,7 @@ func (n *Node) Start() error {
 		running.Protocols = append(running.Protocols, service.Protocols()...)
 	}
 	if err := running.Start(); err != nil {
-		if errno, ok := err.(syscall.Errno); ok && datadirInUseErrnos[uint(errno)] {
-			return ErrDatadirUsed
-		}
-		return err
+		return convertFileLockError(err)
 	}
 	// Start each of the services
 	started := []reflect.Type{}
@@ -242,14 +229,13 @@ func (n *Node) openDataDir() error {
 	if err := os.MkdirAll(instdir, 0700); err != nil {
 		return err
 	}
-	// Try to open the instance directory as LevelDB storage. This creates a lock file
-	// which prevents concurrent use by another instance as well as accidental use of the
-	// instance directory as a database.
-	storage, err := storage.OpenFile(instdir, true)
+	// Lock the instance directory to prevent concurrent use by another instance as well as
+	// accidental use of the instance directory as a database.
+	release, _, err := flock.New(filepath.Join(instdir, "LOCK"))
 	if err != nil {
-		return err
+		return convertFileLockError(err)
 	}
-	n.instanceDirLock = storage
+	n.instanceDirLock = release
 	return nil
 }
 
@@ -509,7 +495,9 @@ func (n *Node) Stop() error {
 
 	// Release instance directory lock.
 	if n.instanceDirLock != nil {
-		n.instanceDirLock.Close()
+		if err := n.instanceDirLock.Release(); err != nil {
+			log.Error("Can't release datadir lock", "err", err)
+		}
 		n.instanceDirLock = nil
 	}