Skip to content

Fix timing issue in parallel execution - #4629

Merged
Jakub Jareš (nohwnd) merged 4 commits into
microsoft:mainfrom
nohwnd:parallel-bug
Aug 2, 2023
Merged

Fix timing issue in parallel execution#4629
Jakub Jareš (nohwnd) merged 4 commits into
microsoft:mainfrom
nohwnd:parallel-bug

Conversation

@nohwnd

Copy link
Copy Markdown
Member

Description

Parallel workers are able to pick up work between setting HasWork and actually associating the workload, which leads to the work being unscheduled. The test run (or test discovery) then never finishes. This was observed on a run with 99 dlls, and can be reproduced locally. To me it was on parallel runs of 12 hosts.

This fix moves HasWork to be the last item to be set under the lock, so the readers that are outside of the lock cannot read it too early before the slot is completely populated.

@nohwnd

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown

No commit pushedDate could be found for PR 4629 in repo microsoft/vstest

@nohwnd

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@nohwnd

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@nohwnd
Jakub Jareš (nohwnd) enabled auto-merge (squash) August 2, 2023 07:33
// This must be set last, every loop below looks at this property,
// and they can do so from a different thread. So if we mark it as HasWork before actually assigning the work
// we can pick up the slot, but it has no associated work yet.
slot.HasWork = true;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The fix.

@Evangelink Amaury Levé (Evangelink) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Linking #4630 here to explain why we approve the change as-is.

@nohwnd
Jakub Jareš (nohwnd) merged commit 9a0c418 into microsoft:main Aug 2, 2023
Jakub Jareš (nohwnd) added a commit to nohwnd/vstest that referenced this pull request Aug 2, 2023
Jakub Jareš (nohwnd) added a commit that referenced this pull request Aug 2, 2023
Co-authored-by: Amaury Levé <amauryleve@microsoft.com>
Fix timing issue in parallel execution (#4629)
This was referenced Aug 10, 2026
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