From cb4023855c5725d79f7b465715adad3ab11280bb Mon Sep 17 00:00:00 2001 From: Ian Davis Date: Wed, 28 Jul 2021 13:08:52 +0100 Subject: [PATCH] Ensure read of alerts slice is performed while holding lock Fixes data race: ================== WARNING: DATA RACE Read at 0x00c029ae7f10 by goroutine 4785: github.com/ipfs/ipfs-cluster.(*Cluster).Alerts() /home/iand/wip/iand/ipfs-cluster/cluster.go:395 +0x64 github.com/ipfs/ipfs-cluster.TestClusterAlerts() /home/iand/wip/iand/ipfs-cluster/ipfscluster_test.go:2159 +0x238 testing.tRunner() /opt/go/src/testing/testing.go:1194 +0x202 Previous write at 0x00c029ae7f10 by goroutine 5062: github.com/ipfs/ipfs-cluster.(*Cluster).alertsHandler() /home/iand/wip/iand/ipfs-cluster/cluster.go:429 +0x48c github.com/ipfs/ipfs-cluster.(*Cluster).run.func5() /home/iand/wip/iand/ipfs-cluster/cluster.go:596 +0x76 Goroutine 4785 (running) created at: testing.(*T).Run() /opt/go/src/testing/testing.go:1239 +0x5d7 testing.runTests.func1() /opt/go/src/testing/testing.go:1512 +0xa6 testing.tRunner() /opt/go/src/testing/testing.go:1194 +0x202 testing.runTests() /opt/go/src/testing/testing.go:1510 +0x612 testing.(*M).Run() /opt/go/src/testing/testing.go:1418 +0x3b3 github.com/ipfs/ipfs-cluster.TestMain() /home/iand/wip/iand/ipfs-cluster/ipfscluster_test.go:134 +0x7dc main.main() _testmain.go:179 +0x271 Goroutine 5062 (running) created at: github.com/ipfs/ipfs-cluster.(*Cluster).run() /home/iand/wip/iand/ipfs-cluster/cluster.go:594 +0x1f6 github.com/ipfs/ipfs-cluster.NewCluster.func1() /home/iand/wip/iand/ipfs-cluster/cluster.go:208 +0xa4 ================== --- FAIL: TestClusterAlerts (8.69s) testing.go:1093: race detected during execution of test --- cluster.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/cluster.go b/cluster.go index 67c56654..276c2d3d 100644 --- a/cluster.go +++ b/cluster.go @@ -45,9 +45,7 @@ const ( maxAlerts = 1000 ) -var ( - errFollowerMode = errors.New("this peer is configured to be in follower mode. Write operations are disabled") -) +var errFollowerMode = errors.New("this peer is configured to be in follower mode. Write operations are disabled") // Cluster is the main IPFS cluster component. It provides // the go-API for it and orchestrates the components that make up the system. @@ -322,7 +320,6 @@ func (c *Cluster) pushInformerMetrics(ctx context.Context, informer Informer) { } metric, err := c.sendInformerMetric(ctx, informer) - if err != nil { if (retries % retryWarnMod) == 0 { logger.Errorf("error broadcasting metric: %s", err) @@ -392,9 +389,8 @@ func (c *Cluster) pushPingMetrics(ctx context.Context) { // Alerts returns the last alerts recorded by this cluster peer with the most // recent first. func (c *Cluster) Alerts() []api.Alert { - alerts := make([]api.Alert, len(c.alerts)) - c.alertsMux.Lock() + alerts := make([]api.Alert, len(c.alerts)) { total := len(alerts) for i, a := range c.alerts { @@ -833,7 +829,7 @@ func (c *Cluster) ID(ctx context.Context) *api.ID { id := &api.ID{ ID: c.id, - //PublicKey: c.host.Peerstore().PubKey(c.id), + // PublicKey: c.host.Peerstore().PubKey(c.id), Addresses: addrs, ClusterPeers: peers, ClusterPeersAddresses: addresses, @@ -1148,7 +1144,6 @@ func (c *Cluster) localPinInfoOp( } // return the last pInfo/err, should be the root Cid if everything ok return pInfo, err - } // RecoverAll triggers a RecoverAllLocal operation on all peers.