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

[R4R] Fix pipecommit active statedb #1002

Merged
merged 2 commits into from
Jul 26, 2022

Conversation

qinglin89
Copy link
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
Collaborator

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
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
Collaborator

@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
Copy link
Collaborator

@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.

None yet

4 participants