fix(dot/state): fix deadlock, fixes bootstrap syncing (ChainSafe#1959)

timwu20 · Nov 3, 2021 · dd80c09 · dd80c09
1 parent 887be9e
commit dd80c09
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 19 deletions.
diff --git a/dot/core/service.go b/dot/core/service.go
@@ -324,7 +324,6 @@ func (s *Service) handleBlocksAsync() {
 			}
 
 			s.maintainTransactionPool(block)
-
 		case <-s.ctx.Done():
 			return
 		}

diff --git a/dot/state/block.go b/dot/state/block.go
@@ -221,6 +221,10 @@ func (bs *BlockState) getAndDeleteUnfinalisedBlock(hash common.Hash) (*types.Blo
 	return block.(*types.Block), true
 }
 
+func (bs *BlockState) deleteUnfinalisedBlock(hash common.Hash) {
+	bs.unfinalisedBlocks.Delete(hash)
+}
+
 // HasHeader returns if the db contains a header with the given hash
 func (bs *BlockState) HasHeader(hash common.Hash) (bool, error) {
 	if bs.hasUnfinalisedBlock(hash) {
@@ -327,6 +331,7 @@ func (bs *BlockState) GetBlockByHash(hash common.Hash) (*types.Block, error) {
 	if err != nil {
 		return nil, err
 	}
+
 	return &types.Block{Header: *header, Body: *blockBody}, nil
 }
 
@@ -360,9 +365,6 @@ func (bs *BlockState) HasBlockBody(hash common.Hash) (bool, error) {
 
 // GetBlockBody will return Body for a given hash
 func (bs *BlockState) GetBlockBody(hash common.Hash) (*types.Body, error) {
-	bs.RLock()
-	defer bs.RUnlock()
-
 	block, has := bs.getUnfinalisedBlock(hash)
 	if has {
 		return &block.Body, nil

diff --git a/dot/state/block_finalisation.go b/dot/state/block_finalisation.go
@@ -226,7 +226,7 @@ func (bs *BlockState) handleFinalisedBlock(curr common.Hash) error {
 			continue
 		}
 
-		block, has := bs.getAndDeleteUnfinalisedBlock(hash)
+		block, has := bs.getUnfinalisedBlock(hash)
 		if !has {
 			return fmt.Errorf("failed to find block in unfinalised block map, block=%s", hash)
 		}
@@ -251,6 +251,8 @@ func (bs *BlockState) handleFinalisedBlock(curr common.Hash) error {
 		if err = batch.Put(headerHashKey(block.Header.Number.Uint64()), hash.ToBytes()); err != nil {
 			return err
 		}
+
+		bs.deleteUnfinalisedBlock(hash)
 	}
 
 	return batch.Flush()

diff --git a/dot/sync/chain_processor.go b/dot/sync/chain_processor.go
@@ -115,11 +115,6 @@ func (s *chainProcessor) processBlockData(bd *types.BlockData) error {
 		return ErrNilBlockData
 	}
 
-	err := s.blockState.CompareAndSetBlockData(bd)
-	if err != nil {
-		return fmt.Errorf("failed to compare and set data: %w", err)
-	}
-
 	hasHeader, _ := s.blockState.HasHeader(bd.Hash)
 	hasBody, _ := s.blockState.HasBlockBody(bd.Hash)
 	if hasHeader && hasBody {
@@ -164,8 +159,10 @@ func (s *chainProcessor) processBlockData(bd *types.BlockData) error {
 		return nil
 	}
 
+	logger.Debug("processing block data", "hash", bd.Hash)
+
 	if bd.Header != nil && bd.Body != nil {
-		if err = s.handleHeader(bd.Header); err != nil {
+		if err := s.handleHeader(bd.Header); err != nil {
 			return err
 		}
 
@@ -176,9 +173,7 @@ func (s *chainProcessor) processBlockData(bd *types.BlockData) error {
 			Body:   *bd.Body,
 		}
 
-		logger.Debug("processing block", "hash", bd.Hash)
-
-		if err = s.handleBlock(block); err != nil {
+		if err := s.handleBlock(block); err != nil {
 			logger.Error("failed to handle block", "number", block.Header.Number, "error", err)
 			return err
 		}
@@ -191,6 +186,10 @@ func (s *chainProcessor) processBlockData(bd *types.BlockData) error {
 		s.handleJustification(bd.Header, *bd.Justification)
 	}
 
+	if err := s.blockState.CompareAndSetBlockData(bd); err != nil {
+		return fmt.Errorf("failed to compare and set data: %w", err)
+	}
+
 	return nil
 }
 

diff --git a/lib/runtime/test_helpers.go b/lib/runtime/test_helpers.go
@@ -89,9 +89,6 @@ func GetRuntimeBlob(testRuntimeFilePath, testRuntimeURL string) (n int64, err er
 	if err != nil {
 		return 0, err
 	}
-	defer func() {
-		_ = out.Close()
-	}()
 
 	/* #nosec */
 	resp, err := http.Get(testRuntimeURL)
@@ -103,7 +100,15 @@ func GetRuntimeBlob(testRuntimeFilePath, testRuntimeURL string) (n int64, err er
 	}()
 
 	n, err = io.Copy(out, resp.Body)
-	return n, err
+	if err != nil {
+		return 0, err
+	}
+
+	if err = out.Close(); err != nil {
+		return 0, err
+	}
+
+	return n, nil
 }
 
 // TestRuntimeNetwork ...

diff --git a/scripts/install-lint.sh b/scripts/install-lint.sh
@@ -6,7 +6,7 @@ fi
 
 if ! command -v golangci-lint &> /dev/null
 then
-	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.41.0
+	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.43.0
 fi
 
 export PATH=$PATH:$(go env GOPATH)/bin