p2p/simulations: revert back to defer Unlock() pattern for Network

It's a good patten to call `defer Unlock()` right after `Lock()` so
(new) error cases won't miss to unlock. Let's get back to that pattern.

The patten was abandoned in 85a79b3ad3,
while fixing a data race. That data race does not exist anymore,
since the Node.Up field got hidden behind its own lock.
This commit is contained in:
Ferenc Szabo 2019-01-31 13:09:36 +01:00
parent addb5b1e6e
commit 83ed2baa63

View file

@ -168,27 +168,23 @@ func (net *Network) Start(id enode.ID) error {
// snapshots // snapshots
func (net *Network) startWithSnapshots(id enode.ID, snapshots map[string][]byte) error { func (net *Network) startWithSnapshots(id enode.ID, snapshots map[string][]byte) error {
net.lock.Lock() net.lock.Lock()
defer net.lock.Unlock()
node := net.getNode(id) node := net.getNode(id)
if node == nil { if node == nil {
net.lock.Unlock()
return fmt.Errorf("node %v does not exist", id) return fmt.Errorf("node %v does not exist", id)
} }
if node.Up() { if node.Up() {
net.lock.Unlock()
return fmt.Errorf("node %v already up", id) return fmt.Errorf("node %v already up", id)
} }
log.Trace("Starting node", "id", id, "adapter", net.nodeAdapter.Name()) log.Trace("Starting node", "id", id, "adapter", net.nodeAdapter.Name())
if err := node.Start(snapshots); err != nil { if err := node.Start(snapshots); err != nil {
net.lock.Unlock()
log.Warn("Node startup failed", "id", id, "err", err) log.Warn("Node startup failed", "id", id, "err", err)
return err return err
} }
node.SetUp(true) node.SetUp(true)
log.Info("Started node", "id", id) log.Info("Started node", "id", id)
ev := NewEvent(node) ev := NewEvent(node)
net.lock.Unlock()
net.events.Send(ev) net.events.Send(ev)
// subscribe to peer events // subscribe to peer events
@ -258,29 +254,23 @@ func (net *Network) watchPeerEvents(id enode.ID, events chan *p2p.PeerEvent, sub
// Stop stops the node with the given ID // Stop stops the node with the given ID
func (net *Network) Stop(id enode.ID) error { func (net *Network) Stop(id enode.ID) error {
net.lock.Lock() net.lock.Lock()
defer net.lock.Unlock()
node := net.getNode(id) node := net.getNode(id)
if node == nil { if node == nil {
net.lock.Unlock()
return fmt.Errorf("node %v does not exist", id) return fmt.Errorf("node %v does not exist", id)
} }
if !node.Up() { if !node.Up() {
net.lock.Unlock()
return fmt.Errorf("node %v already down", id) return fmt.Errorf("node %v already down", id)
} }
node.SetUp(false) node.SetUp(false)
net.lock.Unlock()
err := node.Stop() err := node.Stop()
if err != nil { if err != nil {
net.lock.Lock()
node.SetUp(true) node.SetUp(true)
net.lock.Unlock()
return err return err
} }
log.Info("Stopped node", "id", id, "err", err) log.Info("Stopped node", "id", id, "err", err)
net.lock.Lock()
ev := ControlEvent(node) ev := ControlEvent(node)
net.lock.Unlock()
net.events.Send(ev) net.events.Send(ev)
return nil return nil
} }