From 7e57fee3551ad5b66c985ad208613fd80c2d6b8a Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 18 Aug 2017 12:14:00 +0200 Subject: 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). --- node/node.go | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) (limited to 'node/node.go') 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 } -- cgit