Skip to content

fix(eth): reorder ethereum shutdown sequence #2277

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.

Sign up for GitHub

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

Open
gzliudan wants to merge 1 commit into
base: dev-upgrade
Choose a base branch
from
+22 −1
Open

fix(eth): reorder ethereum shutdown sequence #2277

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 22 additions & 1 deletion eth/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,16 +580,37 @@ func (e *Ethereum) Start() error {
// Stop implements node.Lifecycle, terminating all internal goroutines used by the
// Ethereum protocol.
func (e *Ethereum) Stop() error {
log.Info("Stopping Ethereum bloomIndexer start")
e.bloomIndexer.Close()
log.Info("Ethereum bloomIndexer stopped")
Comment on lines +583 to +585
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

e.bloomIndexer.Close() returns an error, but it’s currently ignored while logs unconditionally report it as “stopped”. Since Ethereum.Stop() has an error return, consider capturing/propagating the close error (or at least logging it) so shutdown failures aren’t silently missed.

Copilot uses AI. Check for mistakes.

log.Info("Stopping Ethereum blockchain start")
e.blockchain.Stop()
log.Info("Ethereum blockchain stopped")

log.Info("Stopping Ethereum protocolManager start")
e.protocolManager.Stop()
log.Info("Ethereum protocolManager stopped")

log.Info("Stopping Ethereum txPool start")
e.txPool.Close()
log.Info("Ethereum txPool stopped")

log.Info("Stopping Ethereum shutdownChan start")
close(e.shutdownChan)
log.Info("Ethereum shutdownChan stopped")

log.Info("Stopping Ethereum miner start")
e.miner.Stop()
log.Info("Ethereum miner stopped")

log.Info("Stopping Ethereum eventMux start")
e.eventMux.Stop()
log.Info("Ethereum eventMux stopped")

log.Info("Stopping Ethereum chainDb start")
e.chainDb.Close()
Comment on lines +599 to 612
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

Closing shutdownChan only signals the bloom service goroutines to exit; it doesn’t ensure they’ve actually stopped before chainDb.Close() runs. In startBloomHandlers the goroutine may still be mid-task (inner for over sections) and continue reading from eth.chainDb. If the intent is to let bloom handlers exit before closing the DB (and avoid shutdown hangs), consider tracking these goroutines with a sync.WaitGroup and waiting after close(e.shutdownChan) (and/or checking shutdownChan inside the per-section loop) prior to closing chainDb.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

e.chainDb.Close() (io.Closer) can return an error, but it’s ignored and the subsequent log line reports the DB as “stopped” regardless. Consider handling/logging the returned error and (optionally) returning it from Ethereum.Stop() so callers can detect shutdown failures.

Suggested change
e.chainDb.Close()
if err := e.chainDb.Close(); err != nil {
log.Error("Failed to stop Ethereum chainDb", "err", err)
return err
}

Copilot uses AI. Check for mistakes.
close(e.shutdownChan)
log.Info("Ethereum chainDb stopped")

return nil
}
Expand Down
Loading