p2p: panic in enode DB Close on shutdown (#9237) #9240
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If any DB method is called while db.Close() is waiting for db.kv.Close() (it waits for ongoing method calls to finish) a panic: "WaitGroup is reused before previous Wait has returned" might happen. Use context cancellation to ensure that new method calls immediately return during db.kv.Close().
AskAlexSharov
requested changes
Jan 16, 2024
There was a problem hiding this comment.
problem is a bit deeper:
panic: sync: WaitGroup is reused before previous Wait has returned
goroutine 508149 [running]:
sync.(*WaitGroup).Wait(0x3?)
sync/waitgroup.go:118 +0x7f
github.com/ledgerwatch/erigon-lib/kv/mdbx.(*MdbxKV).Close(0xc0076199e0?)
github.com/ledgerwatch/[email protected]/kv/mdbx/kv_mdbx.go:497 +0x45
github.com/ledgerwatch/erigon/p2p/enode.(*DB).Close(0xc002708030)
github.com/ledgerwatch/erigon/p2p/enode/nodedb.go:615 +0x65
github.com/ledgerwatch/erigon/p2p.(*Server).Stop(0xc000004c00)
github.com/ledgerwatch/erigon/p2p/server.go:451 +0xed
kv_mdbx.go has WaitGroup+closed atomic bool. Purpose of this fields is: 1. guarantee all active readers are done before close (otherwise may get segfault/race). 2. must be safe to call Close multiple times (underlying env.Close will called once) 3. if db closing started - new readers must be rejected
So, i think we need solve it inside kv_mdbx.go (of-course using context.Background() here is not good also). Then we will not need additional mechanics in each db: txpool/nodedb/downloader/etc...
|
related to #8678 |
|
hmm... not sure if this race is still present or fixed: #8409 |
AskAlexSharov
approved these changes
Jan 16, 2024
This was referenced Jan 16, 2024
battlmonstr
added a commit
that referenced
this pull request
Jan 17, 2024
If any DB method is called while Close() is waiting for db.kv.Close() (it waits for ongoing method calls/transactions to finish) a panic: "WaitGroup is reused before previous Wait has returned" might happen. Use context cancellation to ensure that new method calls immediately return during db.kv.Close().
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If any DB method is called while Close() is waiting for db.kv.Close()
(it waits for ongoing method calls/transactions to finish)
a panic: "WaitGroup is reused before previous Wait has returned" might happen.
Use context cancellation to ensure that new method calls immediately return during db.kv.Close().