Skip to content

[R4R] Fix pipecommit active statedb#1002

Merged
qinglin89 merged 2 commits intobnb-chain:developfrom
qinglin89:fixpipecommitonActiveStatedb
Jul 26, 2022
Merged

[R4R] Fix pipecommit active statedb#1002
qinglin89 merged 2 commits intobnb-chain:developfrom
qinglin89:fixpipecommitonActiveStatedb

Conversation

@qinglin89
Copy link
Copy Markdown
Contributor

Description

prefetcher be accessed after non-nil condition as a field of statedb is not safe.
eg: as in pipecommit mode, statedb might be used as activestatedb to StopPrefetcher, which would panic during:

if prefetcher != nil {
    prefetcher.used(s.data.Root, usedStorage)
 }

Rationale

get prefetcher from statedb before check nil condition

Example

N/A

Changes

@qinglin89 qinglin89 changed the title [R4R] Fixpipecommiton active statedb [R4R] Fixpipecommit active statedb Jul 19, 2022
@brilliant-lx brilliant-lx changed the title [R4R] Fixpipecommit active statedb [R4R] Fix pipecommit active statedb Jul 19, 2022
if s.prefetcher != nil && len(addressesToPrefetch) > 0 {
s.prefetcher.prefetch(s.originalRoot, addressesToPrefetch, emptyAddr)
prefetcher := s.prefetcher
if prefetcher != nil && len(addressesToPrefetch) > 0 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add some UTs to battle test the concurrence issue of statedb?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pointer assignment is actually atomic, but would fail race detection.

@qinglin89 qinglin89 force-pushed the fixpipecommitonActiveStatedb branch from e3fce87 to ec28650 Compare July 20, 2022 03:35
@qinglin89 qinglin89 changed the base branch from develop_July_200M to develop July 20, 2022 03:40
@qinglin89 qinglin89 force-pushed the fixpipecommitonActiveStatedb branch from ec28650 to 9e53b1d Compare July 20, 2022 03:56
brilliant-lx
brilliant-lx previously approved these changes Jul 20, 2022
Copy link
Copy Markdown
Contributor

@brilliant-lx brilliant-lx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

unclezoro
unclezoro previously approved these changes Jul 20, 2022
forcodedancing
forcodedancing previously approved these changes Jul 20, 2022
@qinglin89 qinglin89 force-pushed the fixpipecommitonActiveStatedb branch from 9e53b1d to 4d92fd7 Compare July 22, 2022 10:17
Copy link
Copy Markdown
Contributor

@brilliant-lx brilliant-lx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@qinglin89 qinglin89 merged commit a2a90d3 into bnb-chain:develop Jul 26, 2022
This was referenced Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants