fixes race condition in node stop method

This commit is contained in:
Victor Farazdagi 2017-05-24 17:13:30 +03:00
parent 78865c674b
commit b61f0d0000
6 changed files with 46 additions and 17 deletions

View File

@ -47,12 +47,12 @@ statusgo-ios-simulator-mainnet: xgo
@echo "iOS framework cross compilation done (mainnet)." @echo "iOS framework cross compilation done (mainnet)."
ci: ci:
build/env.sh go test -v -cover ./geth/api build/env.sh go test -v ./geth/api
build/env.sh go test -v -cover ./geth/common build/env.sh go test -v ./geth/common
build/env.sh go test -v -cover ./geth/jail build/env.sh go test -v ./geth/jail
build/env.sh go test -v -cover ./geth/node build/env.sh go test -v ./geth/node
build/env.sh go test -v -cover ./geth/params build/env.sh go test -v ./geth/params
build/env.sh go test -v -cover ./extkeys build/env.sh go test -v ./extkeys
generate: generate:
cp ./node_modules/web3/dist/web3.js ./static/scripts/web3.js cp ./node_modules/web3/dist/web3.js ./static/scripts/web3.js

View File

@ -72,8 +72,6 @@ func (m *StatusBackend) onNodeStart(backendReady chan struct{}, nodeStarted <-ch
if err := m.registerHandlers(); err != nil { if err := m.registerHandlers(); err != nil {
log.Error("Handler registration failed", "err", err) log.Error("Handler registration failed", "err", err)
} }
m.accountManager.ReSelectAccount()
} }
// RestartNode restart running Status node, fails if node is not running // RestartNode restart running Status node, fails if node is not running

View File

@ -83,13 +83,14 @@ func (s *BackendTestSuite) LightEthereumService() *les.LightEthereum {
func (s *BackendTestSuite) RestartTestNode() { func (s *BackendTestSuite) RestartTestNode() {
require := s.Require() require := s.Require()
require.NotNil(s.backend) require.NotNil(s.backend)
require.True(s.backend.IsNodeRunning())
require.True(s.backend.IsNodeRunning()) require.True(s.backend.IsNodeRunning())
nodeRestarted, err := s.backend.RestartNode() nodeRestarted, err := s.backend.RestartNode()
require.NoError(err) require.NoError(err)
require.NotNil(nodeRestarted) require.NotNil(nodeRestarted)
require.True(s.backend.IsNodeRunning())
<-nodeRestarted <-nodeRestarted
require.True(s.backend.IsNodeRunning())
} }
func (s *BackendTestSuite) TestNewBackend() { func (s *BackendTestSuite) TestNewBackend() {

View File

@ -10,6 +10,7 @@ import (
"os" "os"
"path/filepath" "path/filepath"
"reflect" "reflect"
"runtime"
"runtime/debug" "runtime/debug"
"time" "time"
@ -96,6 +97,17 @@ func PanicAfter(waitSeconds time.Duration, abort chan struct{}, desc string) {
}() }()
} }
// NameOf returns name of caller, at runtime
func NameOf(f interface{}) string {
v := reflect.ValueOf(f)
if v.Kind() == reflect.Func {
if rf := runtime.FuncForPC(v.Pointer()); rf != nil {
return rf.Name()
}
}
return v.String()
}
// MessageIDFromContext returns message id from context (if exists) // MessageIDFromContext returns message id from context (if exists)
func MessageIDFromContext(ctx context.Context) string { func MessageIDFromContext(ctx context.Context) string {
if ctx == nil { if ctx == nil {

View File

@ -7,6 +7,7 @@ import (
"os/signal" "os/signal"
"path/filepath" "path/filepath"
"sync" "sync"
"time"
"github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/accounts"
"github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/accounts/keystore"
@ -17,6 +18,7 @@ import (
"github.com/ethereum/go-ethereum/p2p/discover" "github.com/ethereum/go-ethereum/p2p/discover"
"github.com/ethereum/go-ethereum/rpc" "github.com/ethereum/go-ethereum/rpc"
whisper "github.com/ethereum/go-ethereum/whisper/whisperv5" whisper "github.com/ethereum/go-ethereum/whisper/whisperv5"
"github.com/status-im/status-go/geth/common"
"github.com/status-im/status-go/geth/params" "github.com/status-im/status-go/geth/params"
) )
@ -24,6 +26,8 @@ import (
var ( var (
ErrNodeAlreadyExists = errors.New("there is a running node already, stop it before starting another one") ErrNodeAlreadyExists = errors.New("there is a running node already, stop it before starting another one")
ErrNoRunningNode = errors.New("there is no running node") ErrNoRunningNode = errors.New("there is no running node")
ErrNodeOpTimedOut = errors.New("operation takes too long, timed out")
ErrInvalidRunningNode = errors.New("running node is not correctly initialized")
ErrInvalidNodeManager = errors.New("node manager is not properly initialized") ErrInvalidNodeManager = errors.New("node manager is not properly initialized")
ErrInvalidWhisperService = errors.New("whisper service is unavailable") ErrInvalidWhisperService = errors.New("whisper service is unavailable")
ErrInvalidLightEthereumService = errors.New("LES service is unavailable") ErrInvalidLightEthereumService = errors.New("LES service is unavailable")
@ -37,6 +41,7 @@ type NodeManager struct {
sync.RWMutex sync.RWMutex
config *params.NodeConfig // Status node configuration config *params.NodeConfig // Status node configuration
node *node.Node // reference to Geth P2P stack/node node *node.Node // reference to Geth P2P stack/node
nodeStopped chan struct{} // channel to wait for termination notifications
whisperService *whisper.Whisper // reference to Whisper service whisperService *whisper.Whisper // reference to Whisper service
lesService *les.LightEthereum // reference to LES service lesService *les.LightEthereum // reference to LES service
rpcClient *rpc.Client // reference to RPC client rpcClient *rpc.Client // reference to RPC client
@ -78,16 +83,12 @@ func (m *NodeManager) StartNode(config *params.NodeConfig) (<-chan struct{}, err
m.Lock() m.Lock()
defer m.Unlock() defer m.Unlock()
if m.node != nil {
return nil, ErrNodeAlreadyExists
}
return m.startNode(config) return m.startNode(config)
} }
// startNode start Status node, fails if node is already started // startNode start Status node, fails if node is already started
func (m *NodeManager) startNode(config *params.NodeConfig) (<-chan struct{}, error) { func (m *NodeManager) startNode(config *params.NodeConfig) (<-chan struct{}, error) {
if m.node != nil { if m.node != nil || m.nodeStopped != nil {
return nil, ErrNodeAlreadyExists return nil, ErrNodeAlreadyExists
} }
@ -99,6 +100,7 @@ func (m *NodeManager) startNode(config *params.NodeConfig) (<-chan struct{}, err
m.config = config // preserve config of successfully created node m.config = config // preserve config of successfully created node
nodeStarted := make(chan struct{}) nodeStarted := make(chan struct{})
m.nodeStopped = make(chan struct{})
go func() { go func() {
defer HaltOnPanic() defer HaltOnPanic()
@ -108,6 +110,7 @@ func (m *NodeManager) startNode(config *params.NodeConfig) (<-chan struct{}, err
m.lesService = nil m.lesService = nil
m.whisperService = nil m.whisperService = nil
m.rpcClient = nil m.rpcClient = nil
m.nodeStopped = nil
m.node = nil m.node = nil
m.Unlock() m.Unlock()
SendSignal(SignalEnvelope{ SendSignal(SignalEnvelope{
@ -155,6 +158,7 @@ func (m *NodeManager) onNodeStarted(nodeStarted chan struct{}) {
Type: EventNodeStopped, Type: EventNodeStopped,
Event: struct{}{}, Event: struct{}{},
}) })
close(m.nodeStopped)
log.Info("Node is stopped", "enode", nodeInfo.Enode) log.Info("Node is stopped", "enode", nodeInfo.Enode)
} }
@ -167,7 +171,7 @@ func (m *NodeManager) IsNodeRunning() bool {
m.RLock() m.RLock()
defer m.RUnlock() defer m.RUnlock()
return m.node != nil return m.node != nil && m.nodeStopped != nil
} }
// StopNode stop Status node. Stopped node cannot be resumed. // StopNode stop Status node. Stopped node cannot be resumed.
@ -188,16 +192,28 @@ func (m *NodeManager) stopNode() error {
return ErrNoRunningNode return ErrNoRunningNode
} }
if m.nodeStopped == nil { // node may be running, but required channel not set
return ErrInvalidRunningNode
}
if err := m.node.Stop(); err != nil { if err := m.node.Stop(); err != nil {
return err return err
} }
// wait till the previous node is fully stopped
select {
case <-m.nodeStopped:
// pass
case <-time.After(30 * time.Second):
return fmt.Errorf("%v: %s", ErrNodeOpTimedOut, common.NameOf(m.StopNode))
}
m.config = nil m.config = nil
m.lesService = nil m.lesService = nil
m.whisperService = nil m.whisperService = nil
m.rpcClient = nil m.rpcClient = nil
m.nodeStopped = nil
m.node = nil m.node = nil
return nil return nil
} }

View File

@ -304,7 +304,9 @@ func (s *ManagerTestSuite) TestResetChainData() {
time.Sleep(2 * time.Second) // allow to sync for some time time.Sleep(2 * time.Second) // allow to sync for some time
s.True(s.NodeManager.IsNodeRunning()) s.True(s.NodeManager.IsNodeRunning())
require.NoError(s.NodeManager.ResetChainData()) nodeReady, err := s.NodeManager.ResetChainData()
require.NoError(err)
<-nodeReady
s.True(s.NodeManager.IsNodeRunning()) // new node, with previous config should be running s.True(s.NodeManager.IsNodeRunning()) // new node, with previous config should be running
// make sure we can read the first byte, and it is valid (for Rinkeby) // make sure we can read the first byte, and it is valid (for Rinkeby)