Skip to content
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.

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

Fix flakiness TestCancellationViaCommand #243

Merged
merged 1 commit into from
Oct 7, 2021

Conversation

hannahhoward
Copy link
Collaborator

Goals

TestCancellationViaCommand contained a race condition that would cause the response to sometimes finish processing before it could be cancelled.

Implementation

Setup a block hook to hold processing of the response till the CancelResponse call is issued, unblock once command is issued

waitForCancel := make(chan struct{})
td.blockHooks.Register(func(p peer.ID, requestData graphsync.RequestData, blockData graphsync.BlockData, hookActions graphsync.OutgoingBlockHookActions) {
if blkCount == 1 {
<-waitForCancel
Copy link
Collaborator

Choose a reason for hiding this comment

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

so this blocks and we won't increment blkCount until the close on line 121. but nothing checks blkCount directly so this is about blocking the blockHook processing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yea
I can comment to be clear

@hannahhoward hannahhoward force-pushed the fix/flaky-cancellation-via-command branch from 4fb747d to dcccffc Compare October 7, 2021 21:18
Resolve issue that caused TestCancellationViaCommand to sometimes fail as the request was already
finished
@hannahhoward hannahhoward force-pushed the fix/flaky-cancellation-via-command branch from dcccffc to e498932 Compare October 7, 2021 21:21
@hannahhoward hannahhoward merged commit eaccea7 into main Oct 7, 2021
@aschmahmann aschmahmann mentioned this pull request Dec 1, 2021
80 tasks
@mvdan mvdan deleted the fix/flaky-cancellation-via-command branch December 15, 2021 14:18
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