From 83ed2baa633003a67c16c455755e501bd69fcda3 Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Thu, 31 Jan 2019 13:09:36 +0100 Subject: [PATCH] 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 85a79b3ad3c5863f8612d25c246bcfad339f36b7, while fixing a data race. That data race does not exist anymore, since the Node.Up field got hidden behind its own lock. --- p2p/simulations/network.go | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/p2p/simulations/network.go b/p2p/simulations/network.go index e375123211..d373ac88cc 100644 --- a/p2p/simulations/network.go +++ b/p2p/simulations/network.go @@ -168,27 +168,23 @@ func (net *Network) Start(id enode.ID) error { // snapshots func (net *Network) startWithSnapshots(id enode.ID, snapshots map[string][]byte) error { net.lock.Lock() + defer net.lock.Unlock() node := net.getNode(id) if node == nil { - net.lock.Unlock() return fmt.Errorf("node %v does not exist", id) } if node.Up() { - net.lock.Unlock() return fmt.Errorf("node %v already up", id) } log.Trace("Starting node", "id", id, "adapter", net.nodeAdapter.Name()) if err := node.Start(snapshots); err != nil { - net.lock.Unlock() log.Warn("Node startup failed", "id", id, "err", err) return err } node.SetUp(true) log.Info("Started node", "id", id) ev := NewEvent(node) - net.lock.Unlock() - net.events.Send(ev) // 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 func (net *Network) Stop(id enode.ID) error { net.lock.Lock() + defer net.lock.Unlock() node := net.getNode(id) if node == nil { - net.lock.Unlock() return fmt.Errorf("node %v does not exist", id) } if !node.Up() { - net.lock.Unlock() return fmt.Errorf("node %v already down", id) } node.SetUp(false) - net.lock.Unlock() err := node.Stop() if err != nil { - net.lock.Lock() node.SetUp(true) - net.lock.Unlock() return err } log.Info("Stopped node", "id", id, "err", err) - net.lock.Lock() ev := ControlEvent(node) - net.lock.Unlock() net.events.Send(ev) return nil }