Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix concurrent maps modifications and seg faults #1331

Merged
merged 18 commits into from
Feb 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ env:
jobs:
publish-docker:
name: Publish Docker Image
runs-on: ubuntu-18.04
runs-on: ubuntu-20.04
steps:
- name: 'Checkout'
uses: actions/checkout@v2
Expand All @@ -33,7 +33,7 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [ "ubuntu-18.04", "macos-latest" ]
os: [ "ubuntu-20.04", "macos-latest" ]
runs-on: ${{ matrix.os }}
steps:
- name: 'Setup Go ${{ env.GO_VERSION }}'
Expand Down Expand Up @@ -126,7 +126,7 @@ jobs:
needs:
- deploy-cloudsmith
- publish-docker
runs-on: ubuntu-18.04
runs-on: ubuntu-20.04
steps:
- name: 'Check out project files'
uses: actions/checkout@v2
Expand Down Expand Up @@ -179,7 +179,7 @@ jobs:
- deploy-cloudsmith
- publish-docker
- draft-release
runs-on: ubuntu-18.04
runs-on: ubuntu-20.04
steps:
- name: 'Setup metadata'
id: setup
Expand Down
10 changes: 10 additions & 0 deletions core/state/journal.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package state

import (
"math/big"
"sync"

"github.com/ethereum/go-ethereum/common"
)
Expand All @@ -38,6 +39,7 @@ type journalEntry interface {
type journal struct {
entries []journalEntry // Current changes tracked by the journal
dirties map[common.Address]int // Dirty accounts and the number of changes
mutex sync.Mutex
}

// newJournal create a new initialized journal.
Expand All @@ -49,6 +51,8 @@ func newJournal() *journal {

// append inserts a new modification entry to the end of the change journal.
func (j *journal) append(entry journalEntry) {
defer j.mutex.Unlock()
j.mutex.Lock()
j.entries = append(j.entries, entry)
if addr := entry.dirtied(); addr != nil {
j.dirties[*addr]++
Expand All @@ -58,6 +62,8 @@ func (j *journal) append(entry journalEntry) {
// revert undoes a batch of journalled modifications along with any reverted
// dirty handling too.
func (j *journal) revert(statedb *StateDB, snapshot int) {
defer j.mutex.Unlock()
j.mutex.Lock()
for i := len(j.entries) - 1; i >= snapshot; i-- {
// Undo the changes made by the operation
j.entries[i].revert(statedb)
Expand All @@ -76,6 +82,8 @@ func (j *journal) revert(statedb *StateDB, snapshot int) {
// otherwise suggest it as clean. This method is an ugly hack to handle the RIPEMD
// precompile consensus exception.
func (j *journal) dirty(addr common.Address) {
defer j.mutex.Unlock()
j.mutex.Lock()
j.dirties[addr]++
}

Expand Down Expand Up @@ -145,6 +153,8 @@ type (
)

func (ch createObjectChange) revert(s *StateDB) {
defer s.mutex.Unlock()
s.mutex.Lock()
delete(s.stateObjects, *ch.account)
delete(s.stateObjectsDirty, *ch.account)
}
Expand Down
43 changes: 40 additions & 3 deletions core/state/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"
"math/big"
"sort"
"sync"
"time"

"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -74,6 +75,9 @@ type StateDB struct {
snapAccounts map[common.Hash][]byte
snapStorage map[common.Hash]map[common.Hash][]byte

mutex sync.Mutex
journalMutex sync.Mutex

// Quorum - a trie to hold extra account information that cannot be stored in the accounts trie
accountExtraDataTrie Trie

Expand Down Expand Up @@ -332,9 +336,11 @@ func (s *StateDB) Reset(root common.Hash) error {
return err
}
s.trie = tr
s.mutex.Lock()
s.stateObjects = make(map[common.Address]*stateObject)
s.stateObjectsPending = make(map[common.Address]struct{})
s.stateObjectsDirty = make(map[common.Address]struct{})
s.mutex.Unlock()
s.thash = common.Hash{}
s.bhash = common.Hash{}
s.txIndex = 0
Expand Down Expand Up @@ -710,6 +716,8 @@ func (s *StateDB) getDeletedStateObject(addr common.Address) *stateObject {
}

func (s *StateDB) setStateObject(object *stateObject) {
defer s.mutex.Unlock()
s.mutex.Lock()
s.stateObjects[object.Address()] = object
}

Expand Down Expand Up @@ -797,24 +805,38 @@ func (db *StateDB) ForEachStorage(addr common.Address, cb func(key, value common
// Copy creates a deep, independent copy of the state.
// Snapshots of the copied state cannot be applied to the copy.
func (s *StateDB) Copy() *StateDB {
s.journalMutex.Lock()
journal := s.journal
s.journalMutex.Unlock()

journal.mutex.Lock()
size := len(journal.dirties)
dirties := make([]common.Address, 0, size)
for addr := range journal.dirties {
dirties = append(dirties, addr)
}
journal.mutex.Unlock()

// Copy all the basic fields, initialize the memory ones
state := &StateDB{
db: s.db,
trie: s.db.CopyTrie(s.trie),
// Quorum - Privacy Enhancements
accountExtraDataTrie: s.db.CopyTrie(s.accountExtraDataTrie),
stateObjects: make(map[common.Address]*stateObject, len(s.journal.dirties)),
stateObjects: make(map[common.Address]*stateObject, size),
stateObjectsPending: make(map[common.Address]struct{}, len(s.stateObjectsPending)),
stateObjectsDirty: make(map[common.Address]struct{}, len(s.journal.dirties)),
stateObjectsDirty: make(map[common.Address]struct{}, size),
refund: s.refund,
logs: make(map[common.Hash][]*types.Log, len(s.logs)),
logSize: s.logSize,
preimages: make(map[common.Hash][]byte, len(s.preimages)),
journal: newJournal(),
hasher: crypto.NewKeccakState(),
}

s.mutex.Lock()
// Copy the dirty states, logs, and preimages
for addr := range s.journal.dirties {
for _, addr := range dirties {
// As documented [here](https://github.com/ethereum/go-ethereum/pull/16485#issuecomment-380438527),
// and in the Finalise-method, there is a case where an object is in the journal but not
// in the stateObjects: OOG after touch on ripeMD prior to Byzantium. Thus, we need to check for
Expand Down Expand Up @@ -844,6 +866,7 @@ func (s *StateDB) Copy() *StateDB {
}
state.stateObjectsDirty[addr] = struct{}{}
}
s.mutex.Unlock()
for hash, logs := range s.logs {
cpy := make([]*types.Log, len(logs))
for i, l := range logs {
Expand Down Expand Up @@ -930,7 +953,13 @@ func (s *StateDB) GetRefund() uint64 {
// into the tries just yet. Only IntermediateRoot or Commit will do that.
func (s *StateDB) Finalise(deleteEmptyObjects bool) {
addressesToPrefetch := make([][]byte, 0, len(s.journal.dirties))
s.journal.mutex.Lock()
dirties := make([]common.Address, 0, len(s.journal.dirties))
for addr := range s.journal.dirties {
dirties = append(dirties, addr)
}
s.journal.mutex.Unlock()
for _, addr := range dirties {
obj, exist := s.stateObjects[addr]
if !exist {
// ripeMD is 'touched' at block 1714175, in tx 0x1237f737031e40bcde4a8b7e717b2d15e3ecadfe49bb1bbc71ee9deb09c6fcf2
Expand All @@ -956,8 +985,10 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool) {
} else {
obj.finalise(true) // Prefetch slots in the background
}
s.mutex.Lock()
s.stateObjectsPending[addr] = struct{}{}
s.stateObjectsDirty[addr] = struct{}{}
s.mutex.Unlock()

// At this point, also ship the address off to the precacher. The precacher
// will start loading tries, and when the change is eventually committed,
Expand All @@ -978,6 +1009,8 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash {
// Finalise all the dirty storage states and write them into the tries
s.Finalise(deleteEmptyObjects)

s.mutex.Lock()
defer s.mutex.Unlock()
// If there was a trie prefetcher operating, it gets aborted and irrevocably
// modified after we start retrieving tries. Remove it from the statedb after
// this round of use.
Expand Down Expand Up @@ -1042,6 +1075,8 @@ func (s *StateDB) Prepare(thash, bhash common.Hash, ti int) {
}

func (s *StateDB) clearJournalAndRefund() {
defer s.journalMutex.Unlock()
s.journalMutex.Lock()
if len(s.journal.entries) > 0 {
s.journal = newJournal()
s.refund = 0
Expand All @@ -1059,6 +1094,7 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (common.Hash, error) {
// Finalize any pending changes and merge everything into the tries
s.IntermediateRoot(deleteEmptyObjects)

s.mutex.Lock()
// Commit objects to the trie, measuring the elapsed time
codeWriter := s.db.TrieDB().DiskDB().NewBatch()
for addr := range s.stateObjectsDirty {
Expand All @@ -1077,6 +1113,7 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (common.Hash, error) {
if len(s.stateObjectsDirty) > 0 {
s.stateObjectsDirty = make(map[common.Address]struct{})
}
s.mutex.Unlock()
if codeWriter.ValueSize() > 0 {
if err := codeWriter.Write(); err != nil {
log.Crit("Failed to commit dirty codes", "error", err)
Expand Down
11 changes: 11 additions & 0 deletions eth/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"errors"
"fmt"
"math/big"
"os"
"runtime"
"sync"
"sync/atomic"
Expand Down Expand Up @@ -238,6 +239,16 @@ func New(stack *node.Node, config *ethconfig.Config) (*Ethereum, error) {
if err != nil {
return nil, err
}
defer func() {
if p := recover(); p != nil {
log.Error("panic occurred", "err", p)
err := eth.Stop()
if err != nil {
log.Error("error while closing", "err", err)
}
os.Exit(1)
}
}()
// Rewind the chain in case of an incompatible config upgrade.
if compat, ok := genesisErr.(*params.ConfigCompatError); ok {
log.Warn("Rewinding chain to upgrade configuration", "err", compat)
Expand Down