From 0cd3493fa5b060ce7a634eec53f2df2165e9ed13 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 2 Dec 2019 22:36:59 +0300 Subject: [PATCH 1/2] core: fix potential locking problem in mempool I think it should fix this issue mentioned in the #526: INFO[1456] blockchain persist completed blockHeight=63480 headerHeight=1810000 persistedBlocks=1 persistedKeys=4 took=740.7113ms fatal error: concurrent map read and map write goroutine 322 [running]: runtime.throw(0xc8a6dc, 0x21) /usr/local/go/src/runtime/panic.go:774 +0x72 fp=0xc003473958 sp=0xc003473928 pc=0x42e282 runtime.mapaccess2(0xb706a0, 0xc0001893b0, 0xc0034739c8, 0xc0028704e0, 0x3) /usr/local/go/src/runtime/map.go:470 +0x278 fp=0xc0034739a0 sp=0xc003473958 pc=0x40dc08 github.com/CityOfZion/neo-go/pkg/core.MemPool.ContainsKey(0xc0001d0d20, 0xc0001893b0, 0xc0001893e0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...) /neo-go/pkg/core/mem_pool.go:92 +0xcb fp=0xc003473a30 sp=0xc0034739a0 pc=0x9326db github.com/CityOfZion/neo-go/pkg/core.(*Blockchain).HasTransaction(0xc0001ca8c0, 0x49f0b45d430e441b, 0x553db79b7072821c, 0x28969518de11976, 0xba5100efddbe79d4, 0xc003473cd0) /neo-go/pkg/core/blockchain.go:803 +0xa1 fp=0xc003473b68 sp=0xc003473a30 pc=0x914b11 github.com/CityOfZion/neo-go/pkg/core.Blockchainer.HasTransaction-fm(0x49f0b45d430e441b, 0x553db79b7072821c, 0x28969518de11976, 0xba5100efddbe79d4, 0xc005a5d388) /neo-go/pkg/core/blockchainer.go:28 +0x46 fp=0xc003473ba8 sp=0xc003473b68 pc=0x997326 github.com/CityOfZion/neo-go/pkg/network.(*Server).handleInvCmd(0xc00018f680, 0xd9d980, 0xc00025e190, 0xc005a5d380, 0x0, 0x0) /neo-go/pkg/network/server.go:401 +0x3bb fp=0xc003473d38 sp=0xc003473ba8 pc=0x9924cb github.com/CityOfZion/neo-go/pkg/network.(*Server).handleMessage(0xc00018f680, 0xd9d980, 0xc00025e190, 0xc007a0d050, 0x0, 0x0) /neo-go/pkg/network/server.go:582 +0x1ae fp=0xc003473da0 sp=0xc003473d38 pc=0x993bbe github.com/CityOfZion/neo-go/pkg/network.(*TCPTransport).handleConn(0xc000228420, 0xd9b880, 0xc0001b6f00) /neo-go/pkg/network/tcp_transport.go:93 +0x202 fp=0xc003473fc8 sp=0xc003473da0 pc=0x996672 runtime.goexit() /usr/local/go/src/runtime/asm_amd64.s:1357 +0x1 fp=0xc003473fd0 sp=0xc003473fc8 pc=0x45b3e1 created by github.com/CityOfZion/neo-go/pkg/network.(*TCPTransport).Dial /neo-go/pkg/network/tcp_transport.go:36 +0xb4 The problem is that we're modifying `unsortedTxn` under a reader lock. --- pkg/core/mem_pool.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/core/mem_pool.go b/pkg/core/mem_pool.go index 2b710a883..8af3bdc35 100644 --- a/pkg/core/mem_pool.go +++ b/pkg/core/mem_pool.go @@ -104,13 +104,13 @@ func (mp MemPool) ContainsKey(hash util.Uint256) bool { func (mp MemPool) TryAdd(hash util.Uint256, pItem *PoolItem) bool { var pool PoolItems - mp.lock.RLock() + mp.lock.Lock() if _, ok := mp.unsortedTxn[hash]; ok { - mp.lock.RUnlock() + mp.lock.Unlock() return false } mp.unsortedTxn[hash] = pItem - mp.lock.RUnlock() + mp.lock.Unlock() if pItem.fee.IsLowPriority(pItem.txn) { pool = mp.sortedLowPrioTxn From 1c89c192acb62bced4dca21aa82b6b7e27ec27c0 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 2 Dec 2019 22:39:43 +0300 Subject: [PATCH 2/2] core: optimize some accesses to unsortedTxn in mempool These don't need a full lock, they only read things from maps. --- pkg/core/mem_pool.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/core/mem_pool.go b/pkg/core/mem_pool.go index 8af3bdc35..52611dc74 100644 --- a/pkg/core/mem_pool.go +++ b/pkg/core/mem_pool.go @@ -226,8 +226,8 @@ func NewMemPool(capacity int) MemPool { // TryGetValue returns a transaction if it exists in the memory pool. func (mp MemPool) TryGetValue(hash util.Uint256) (*transaction.Transaction, bool) { - mp.lock.Lock() - defer mp.lock.Unlock() + mp.lock.RLock() + defer mp.lock.RUnlock() if pItem, ok := mp.unsortedTxn[hash]; ok { return pItem.txn, ok } @@ -271,8 +271,8 @@ func min(sortedPool PoolItems) *PoolItem { func (mp *MemPool) GetVerifiedTransactions() []*transaction.Transaction { var t []*transaction.Transaction - mp.lock.Lock() - defer mp.lock.Unlock() + mp.lock.RLock() + defer mp.lock.RUnlock() for _, p := range mp.unsortedTxn { t = append(t, p.txn) }