Skip to content

chore(cmd/XDC): drop the unreachable CheckpointCh close - #2576

Open
gzliudan wants to merge 1 commit into
XinFinOrg:dev-upgradefrom
gzliudan:drop-CheckpointCh-close
Open

gzliudan wants to merge 1 commit into
XinFinOrg:dev-upgradefrom
gzliudan:drop-CheckpointCh-close

Conversation

@gzliudan

Copy link
Copy Markdown
Collaborator

Problem

startNode starts a goroutine that validates this node's masternode and then reconciles the staking state at every epoch switch. Its last two statements are adjacent:

defer close(core.CheckpointCh)
for range core.CheckpointCh {

The deferred close can never run. A defer runs when the function returns, and nothing after it can return: the for range that follows is the only remaining statement and it consumes signals until the channel is closed and drained. Reaching the defer therefore requires the channel to be closed already - by which point a second close would be a different failure anyway. It is dead code, and it has been since the loop was written.

What makes it worth removing is that it states a guarantee the surrounding code does not provide. core.CheckpointCh is a package level channel whose senders outlive startNode and everything in it:

  • core/blockchain.go sends on every epoch switch block reaching the head, from both insertChain and insertBlock, with the chain mutex held;
  • miner/worker.go sends from the goroutine that drains the mined-block queue.

None of them is aware of this goroutine's lifetime. Any future change that gives this loop an exit - a shutdown signal, an error return - would turn the defer into a race, and a close concurrent with one of those sends panics with "send on closed channel". Leaving the close there invites exactly that change; leaving nothing there states the truth, which is that this channel lives as long as the process and is only ever drained.

Introduced together with the loop in 9f5cba7dc ("update new set of masternodes at end of each epoch (distance = m1Gap)", 2018-09-30), and never touched since.

Fix

Drop the defer and replace it with a comment recording why the channel is deliberately never closed: who sends on it, that this loop is its only reader, and that a close could only race a send. No sender, no receiver, and no behaviour changes - the statement removed could never execute.

Verification

  • gofmt clean; go build ./... and go vet ./cmd/XDC pass.
  • No test is added. The removed statement is unreachable, so no observation - including a test - can tell before from after; cmd/XDC is not covered by the test suite in any case. The change is confined to one file in cmd/XDC, so nothing else needed re-verification.

Attribution

A pre-existing cleanup, found while reading the checkpoint plumbing for #2534 and #2535; it is unrelated to those two issues and does not depend on any work in flight.

The same cleanup has also been carried on the #2534 / #2535 line of work (de396c3a7 on fix-issue-2534-2535, opened as #2566) so that branch could keep signalling checkpoints safely. Whichever of the two lands first, the other should drop its copy on rebase - they touch the same line.

A deferred close only runs when the goroutine returns, and the for-range
right below it is the channel's only reader: it can never return. Sends
from the block import paths and from miner/worker.go would race the
close if it ever did.
@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: a3d02c95-a167-4454-bf14-150edc452b90

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

2 participants