Merge pull request #7029 from MStreet3/bug/blockcache-test-data-race

blockcache: fix datarace in blockcache_test

blockcache/blockcache_test.go

@@ -20,16 +20,18 @@ type mockChainBackend struct {
 	sync.RWMutex
 }
 
-func (m *mockChainBackend) addBlock(block *wire.MsgBlock, nonce uint32) {
+func newMockChain() *mockChainBackend {
+	return &mockChainBackend{
+		blocks: make(map[chainhash.Hash]*wire.MsgBlock),
+	}
+}
+
+// GetBlock is a mock implementation of block fetching that tracks the number
+// of backend calls and returns the block found for the given hash or an error.
+func (m *mockChainBackend) GetBlock(blockHash *chainhash.Hash) (*wire.MsgBlock, error) {
 	m.Lock()
 	defer m.Unlock()
-	block.Header.Nonce = nonce
+
-	hash := block.Header.BlockHash()
-	m.blocks[hash] = block
-}
-func (m *mockChainBackend) GetBlock(blockHash *chainhash.Hash) (*wire.MsgBlock, error) {
-	m.RLock()
-	defer m.RUnlock()
 	m.chainCallCount++
 
 	block, ok := m.blocks[*blockHash]
@@ -40,15 +42,25 @@ func (m *mockChainBackend) GetBlock(blockHash *chainhash.Hash) (*wire.MsgBlock,
 	return block, nil
 }
 
-func newMockChain() *mockChainBackend {
+func (m *mockChainBackend) getChainCallCount() int {
-	return &mockChainBackend{
+	m.RLock()
-		blocks: make(map[chainhash.Hash]*wire.MsgBlock),
+	defer m.RUnlock()
-	}
+
+	return m.chainCallCount
+}
+
+func (m *mockChainBackend) addBlock(block *wire.MsgBlock, nonce uint32) {
+	m.Lock()
+	defer m.Unlock()
+
+	block.Header.Nonce = nonce
+	hash := block.Header.BlockHash()
+	m.blocks[hash] = block
 }
 
 func (m *mockChainBackend) resetChainCallCount() {
-	m.RLock()
+	m.Lock()
-	defer m.RUnlock()
+	defer m.Unlock()
 
 	m.chainCallCount = 0
 }
@@ -90,7 +102,7 @@ func TestBlockCacheGetBlock(t *testing.T) {
 	_, err := bc.GetBlock(&blockhash1, getBlockImpl)
 	require.NoError(t, err)
 	require.Equal(t, 1, bc.Cache.Len())
-	require.Equal(t, 1, mc.chainCallCount)
+	require.Equal(t, 1, mc.getChainCallCount())
 	mc.resetChainCallCount()
 
 	_, err = bc.Cache.Get(*inv1)
@@ -102,7 +114,7 @@ func TestBlockCacheGetBlock(t *testing.T) {
 	_, err = bc.GetBlock(&blockhash2, getBlockImpl)
 	require.NoError(t, err)
 	require.Equal(t, 2, bc.Cache.Len())
-	require.Equal(t, 1, mc.chainCallCount)
+	require.Equal(t, 1, mc.getChainCallCount())
 	mc.resetChainCallCount()
 
 	_, err = bc.Cache.Get(*inv1)
@@ -117,7 +129,7 @@ func TestBlockCacheGetBlock(t *testing.T) {
 	_, err = bc.GetBlock(&blockhash1, getBlockImpl)
 	require.NoError(t, err)
 	require.Equal(t, 2, bc.Cache.Len())
-	require.Equal(t, 0, mc.chainCallCount)
+	require.Equal(t, 0, mc.getChainCallCount())
 	mc.resetChainCallCount()
 
 	// Since the Cache is now at its max capacity, it is expected that when
@@ -128,7 +140,7 @@ func TestBlockCacheGetBlock(t *testing.T) {
 	_, err = bc.GetBlock(&blockhash3, getBlockImpl)
 	require.NoError(t, err)
 	require.Equal(t, 2, bc.Cache.Len())
-	require.Equal(t, 1, mc.chainCallCount)
+	require.Equal(t, 1, mc.getChainCallCount())
 	mc.resetChainCallCount()
 
 	_, err = bc.Cache.Get(*inv1)
@@ -183,5 +195,5 @@ func TestBlockCacheMutexes(t *testing.T) {
 	}
 
 	wg.Wait()
-	require.Equal(t, 2, mc.chainCallCount)
+	require.Equal(t, 2, mc.getChainCallCount())
 }

docs/release-notes/release-notes-0.16.0.md

@@ -76,6 +76,9 @@ crash](https://github.com/lightningnetwork/lnd/pull/7019).
 * [Fixed potential data race on funding manager
   restart](https://github.com/lightningnetwork/lnd/pull/6929).
 
+* [Fixed a flake in the TestBlockCacheMutexes unit
+  test](https://github.com/lightningnetwork/lnd/pull/7029).
+
 ## `lncli`
 * [Add an `insecure` flag to skip tls auth as well as a `metadata` string slice
   flag](https://github.com/lightningnetwork/lnd/pull/6818) that allows the
@@ -153,6 +156,7 @@ crash](https://github.com/lightningnetwork/lnd/pull/7019).
 * hieblmi
 * Jesse de Wit
 * Matt Morehouse
+* Michael Street
 * Olaoluwa Osuntokun
 * Oliver Gugger
 * Priyansh Rastogi