From fecf2141750fe61640870672987023fe7213901a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Tue, 9 Jun 2015 21:02:26 +0300 Subject: [PATCH 1/3] core: fix a lock annoyance and potential deadlock --- core/chain_manager.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/core/chain_manager.go b/core/chain_manager.go index c69d3a10e..82fdbb1f1 100644 --- a/core/chain_manager.go +++ b/core/chain_manager.go @@ -376,6 +376,8 @@ func (self *ChainManager) ExportN(w io.Writer, first uint64, last uint64) error return nil } +// insert appends injects a block into the current chain block chain. Note, this +// function assumes that the `mu` mutex is held! func (bc *ChainManager) insert(block *types.Block) { key := append(blockNumPre, block.Number().Bytes()...) bc.blockDb.Put(key, block.Hash().Bytes()) @@ -484,6 +486,8 @@ func (self *ChainManager) GetAncestors(block *types.Block, length int) (blocks [ return } +// setTotalDifficulty updates the TD of the chain manager. Note, this function +// assumes that the `mu` mutex is held! func (bc *ChainManager) setTotalDifficulty(td *big.Int) { bc.td = new(big.Int).Set(td) } @@ -540,9 +544,6 @@ func (self *ChainManager) InsertChain(chain types.Blocks) (int, error) { self.wg.Add(1) defer self.wg.Done() - self.mu.Lock() - defer self.mu.Unlock() - self.chainmu.Lock() defer self.chainmu.Unlock() @@ -625,7 +626,7 @@ func (self *ChainManager) InsertChain(chain types.Blocks) (int, error) { cblock := self.currentBlock // Compare the TD of the last known block in the canonical chain to make sure it's greater. // At this point it's possible that a different chain (fork) becomes the new canonical chain. - if block.Td.Cmp(self.td) > 0 { + if block.Td.Cmp(self.Td()) > 0 { // chain fork if block.ParentHash() != cblock.Hash() { // during split we merge two different chains and create the new canonical chain @@ -638,8 +639,10 @@ func (self *ChainManager) InsertChain(chain types.Blocks) (int, error) { queueEvent.splitCount++ } + self.mu.Lock() self.setTotalDifficulty(block.Td) self.insert(block) + self.mu.Unlock() jsonlogger.LogJson(&logger.EthChainNewHead{ BlockHash: block.Hash().Hex(), @@ -747,9 +750,11 @@ func (self *ChainManager) merge(oldBlock, newBlock *types.Block) error { } // insert blocks. Order does not matter. Last block will be written in ImportChain itself which creates the new head properly + self.mu.Lock() for _, block := range newChain { self.insert(block) } + self.mu.Unlock() return nil } From d652a58ada207fa9a372f2fa594d5a151ed44a4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Tue, 9 Jun 2015 21:13:21 +0300 Subject: [PATCH 2/3] core: fix a race condition accessing the gas limit --- core/chain_manager.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/chain_manager.go b/core/chain_manager.go index 82fdbb1f1..6897c453c 100644 --- a/core/chain_manager.go +++ b/core/chain_manager.go @@ -179,7 +179,9 @@ func (self *ChainManager) Td() *big.Int { } func (self *ChainManager) GasLimit() *big.Int { - // return self.currentGasLimit + self.mu.RLock() + defer self.mu.RUnlock() + return self.currentBlock.GasLimit() } @@ -376,8 +378,8 @@ func (self *ChainManager) ExportN(w io.Writer, first uint64, last uint64) error return nil } -// insert appends injects a block into the current chain block chain. Note, this -// function assumes that the `mu` mutex is held! +// insert injects a block into the current chain block chain. Note, this function +// assumes that the `mu` mutex is held! func (bc *ChainManager) insert(block *types.Block) { key := append(blockNumPre, block.Number().Bytes()...) bc.blockDb.Put(key, block.Hash().Bytes()) From 4541c2296441ba7e7e625d95d45490dbd064c8b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Tue, 9 Jun 2015 21:33:39 +0300 Subject: [PATCH 3/3] event/filter: hack around data race in the test --- event/filter/filter_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/event/filter/filter_test.go b/event/filter/filter_test.go index 815deb63a..534eb56d1 100644 --- a/event/filter/filter_test.go +++ b/event/filter/filter_test.go @@ -1,6 +1,9 @@ package filter -import "testing" +import ( + "testing" + "time" +) func TestFilters(t *testing.T) { var success bool @@ -24,6 +27,8 @@ func TestFilters(t *testing.T) { fm.Notify(Generic{Str1: "hello"}, true) fm.Stop() + time.Sleep(10 * time.Millisecond) // yield to the notifier + if !success { t.Error("expected 'hello' to be posted") }