Skip to content

Fix node message forwarding race#7942

Merged
achamayou merged 2 commits into
microsoft:mainfrom
maxtropets:f/consensus-ptr-race
Jun 15, 2026
Merged

Fix node message forwarding race#7942
achamayou merged 2 commits into
microsoft:mainfrom
maxtropets:f/consensus-ptr-race

Conversation

@maxtropets

@maxtropets maxtropets commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Failed in https://github.com/microsoft/CCF/actions/runs/27516961192/job/81327255271

As per #7936, there's early consensus.ptr() deref, which is racing with setup_consensus().

@maxtropets maxtropets self-assigned this Jun 15, 2026
Copilot AI review requested due to automatic review settings June 15, 2026 14:33
@maxtropets maxtropets requested a review from a team as a code owner June 15, 2026 14:33

Copilot AI left a comment

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.

Pull request overview

This PR adjusts node-to-node inbound message handling to avoid accessing handler pointers (notably consensus) during early startup states, addressing the race described in #7936 by gating message processing before dispatch.

Changes:

  • Move the “can process inbound node message” gate to NodeState::recv_node_inbound() so handler pointers aren’t touched until the node is in an active state.
  • Add can_process_node_inbound_message(sm) helper and update recv_node_inbound_message to assume the caller has already gated.
  • Update the C++ unit test to validate the gate predicate separately from dispatch behaviour.

Custom instructions used:

  • .github/copilot-instructions.md
  • .github/instructions/reviewing.instructions.md

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/node/test/node_inbound_message.cpp Refactors the unit test to check gating predicate separately and verify dispatch for each message type.
src/node/node_state.h Adds the startup-state gate in recv_node_inbound before calling into the dispatcher, preventing early handler pointer access.
src/node/node_inbound_message.h Introduces can_process_node_inbound_message helper and removes internal gating from recv_node_inbound_message.

Comment thread src/node/node_inbound_message.h
@achamayou achamayou merged commit 3cc6a36 into microsoft:main Jun 15, 2026
19 checks passed
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.

3 participants