From dd80c09670b076cfbaa1bda2ec0c8b2d83253ad7 Mon Sep 17 00:00:00 2001 From: noot <36753753+noot@users.noreply.github.com> Date: Wed, 3 Nov 2021 16:54:25 -0400 Subject: [PATCH] fix(dot/state): fix deadlock, fixes bootstrap syncing (#1959) --- dot/core/service.go | 1 - dot/state/block.go | 8 +++++--- dot/state/block_finalisation.go | 4 +++- dot/sync/chain_processor.go | 17 ++++++++--------- lib/runtime/test_helpers.go | 13 +++++++++---- scripts/install-lint.sh | 2 +- 6 files changed, 26 insertions(+), 19 deletions(-) diff --git a/dot/core/service.go b/dot/core/service.go index 289881e8dc..f79dd56f5e 100644 --- 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 index 46fc641b46..09632c733a 100644 --- 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 index aaeb9ffcfc..2e85ac37d4 100644 --- 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 index 9333a4e600..bc7972ea45 100644 --- 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 index c6e969e69d..59da6fd68d 100644 --- 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 index 30b7c6baaa..bd1e4cf175 100755 --- 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 \ No newline at end of file