Skip to content

Don't let TestTryCommittedSequenceTwice hang when the double delivery race doesn't happen - #719

Merged
pfi79 merged 1 commit into
hyperledger:mainfrom
HagarMeir:fix/try-committed-sequence-twice-hang
Oct 22, 2026
Merged

pfi79 merged 1 commit into
hyperledger:mainfrom
HagarMeir:fix/try-committed-sequence-twice-hang

Conversation

@HagarMeir

Copy link
Copy Markdown
Contributor

Written by Claude (Anthropic's AI assistant) on behalf of Hagar Meir.

Fixes #718.

The test's log hooks on node 4 waited forever for both the controller and the view changer to deliver the same decision. When node 4 decided it through the controller only, the controller blocked in the hook, and the test hung until the package timeout.

Changes (test only)

  • Each wait in the hook gives up after 5 seconds and logs that the race didn't happen.
  • deliverControlCh is closed at most once.
  • Sends to twiceDeliverBeginCh don't block once both goroutines have signalled.

The final assertion, that all nodes delivered the same value, is unchanged, so a double delivery or a fork still fails the test.

Testing

  • 100 of 100 runs of the test passed. In 7 of them the race didn't happen and the bounded wait was used; those took 15–27s instead of about 10s.
  • 20 of 20 runs passed with -race.
  • For comparison, on main the test hung in 1 of 60 runs.
  • After rebasing onto main with add golangci-lint check #717: 30 of 30 runs passed, the full test package passed, and golangci-lint run ./... reports no issues.

🤖 Generated with Claude Code

@HagarMeir
HagarMeir force-pushed the fix/try-committed-sequence-twice-hang branch from b5a985d to b6914ac Compare October 22, 2026 07:04
…ppen

The test's log hooks on node 4 waited forever for both the controller
and the view changer to deliver the same decision. When node 4 decided
it through the controller only, the controller blocked in the hook and
the test hung until the package timeout.

Wait for the other goroutine only for a while, close deliverControlCh at
most once, and don't block on twiceDeliverBeginCh once both goroutines
signalled. The final check that all nodes delivered the same value is
unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Hagar Meir <hagar.meir@ibm.com>
@pfi79
pfi79 force-pushed the fix/try-committed-sequence-twice-hang branch from b6914ac to bd88e95 Compare October 22, 2026 07:09
@pfi79
pfi79 merged commit a79c785 into hyperledger:main Oct 22, 2026
6 checks passed
@HagarMeir
HagarMeir deleted the fix/try-committed-sequence-twice-hang branch October 22, 2026 17:23
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.

TestTryCommittedSequenceTwice sometimes hangs until the test timeout

2 participants