Skip to content

Fix flaky CanShutdownServerProcess test#7871

Merged
rokonec merged 14 commits intodotnet:mainfrom
rokonec:rokonec/fix-flaky-msb-server-test
Aug 14, 2022
Merged

Fix flaky CanShutdownServerProcess test#7871
rokonec merged 14 commits intodotnet:mainfrom
rokonec:rokonec/fix-flaky-msb-server-test

Conversation

@rokonec
Copy link
Copy Markdown
Member

@rokonec rokonec commented Aug 9, 2022

Context

We detected flaky unit tests related to msbuild server shutdown.
Example of failing checks: #7835

Root cause:
When server finish its build it eventually drops namedpipe, open new one with same name and start listen on it.
If clients is trying to connect to server during this brief time, all kinds of exceptions could happen.

Changes Made

a) Previously named mutex server-is-running was by server dropped after project weas build and re-acquired shortly after. Changes was made so mutex is acquired constantly.
b) Client retries during connection to server, which address all weird race conditions when server recycling named-pipe.

Testing

Several /azp run s
Can't repro on Linux and MacOS even though I could before final changes.

Notes

Same bug is also possible while connecting to working nodes. In such case though, the worst that could happen though is that we span one unnecessary node which is, considering low probability of this to happen, acceptable.

@rokonec rokonec changed the title Wait a bit on non Windows OS Fix flaky CanShutdownServerProcess test Aug 9, 2022
@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 9, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 9, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 9, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 9, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 9, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 9, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@Forgind
Copy link
Copy Markdown
Contributor

Forgind commented Aug 9, 2022

The current test failure is from OldNugetTest. Are we using the server by default now? If so, it may be creating a server process, then expecting it to exit, and it doesn't (nor should it). If that's it, we may need to disable the server for that test.

@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 9, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 10, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec rokonec changed the title Fix flaky CanShutdownServerProcess test [WIP] Fix flaky CanShutdownServerProcess test Aug 10, 2022
@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 10, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 10, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 10, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 10, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 11, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 11, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 11, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 11, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 11, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec rokonec changed the title [WIP] Fix flaky CanShutdownServerProcess test Fix flaky CanShutdownServerProcess test Aug 11, 2022
Copy link
Copy Markdown
Contributor

@Forgind Forgind left a comment

Choose a reason for hiding this comment

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

Looks great! I'm assuming the test passed for all the retries you did?

@rokonec rokonec added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Aug 12, 2022
@rokonec
Copy link
Copy Markdown
Member Author

rokonec commented Aug 13, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rokonec rokonec merged commit 50f6081 into dotnet:main Aug 14, 2022
@rokonec rokonec deleted the rokonec/fix-flaky-msb-server-test branch August 14, 2022 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants