-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
dotnet msbuild
commands sometimes hang after writing MSBuild_*.failure.txt files
#1808
Comments
FYI @Eilon this is the hang we discussed a couple of days back. |
Behaviour more consistent after I rebased
I tested in the |
Just saw a message about
and / or (or is much more common)
Of course, these failures should not result in hangs. |
@pranavkm please add your smaller (IdentityServer) repro steps to this bug. |
dotnet msbuild
commands hang with some command-line redirectionsdotnet msbuild
commands sometimes hang after writing MSBuild_*.failure.txt files
Renamed issue because redirection is not required to see the hangs. |
/cc @piotrpMSFT since you put the old bug into a milestone. |
Saw this in our CI and locally recently when building this specific commit https://github.com/aspnet/IdentityService/commit/7f3707520cc5bba05947ef8a660b3c41eb5968a7:
MSBuild prints this error and ends up hanging. Our CI eventually killed it after 240 minutes of inactivity: Failure log file says:
|
This happened again today with MSBuild 15.2.47.30403 on .NET Core. I've gathered process dumps of MSBuild in its hung state. Let me know if you'd like me to email them to you. |
Simplified Repro Steps
|
This patch addresses a fatal error and potential node crash in MSBuild in a very specific condition. Consider project A builds project B, C, B (using MSBuild task), project B builds project C, and project C produces an error. In a multi-proc build, the master node may build A, start building B and yield execution for C to be built (on another node). On another node, the dependencies of B are built and failed, returning the results to the master node. When the execution of B resumes, the failure occurs trying to set the result of the error target (target result already exists). This change calls the CheckSkipTarget method to determine if the target has already been built in the case of an error. Previously this check was made when checking dependencies of the target to inject or executing the target. I believe this was an oversight in the original change that was made to address this issue in a previous release of Visual Studio. Note: The scenario described is implemented in unit test VerifyMultipleRequestForSameProjectWithErrors_ErrorAndContinue. However, since this issue causes a node to crash the unit test written did not detect the more serious exception since the build was intended to fail anyway. With this change, the test now passes. Closes dotnet#1808
Addresses an issue in VerifyMultipleRequestForSameProjectWithErrors_ErrorAndContinue where the test will fail (writing an MSBuild_*.failure.txt file in the temp folder) but is not detected because a failure was expected. The primary change is to detect the number of MSBuild_*.failure.txt files and assert the value is unchanged before and after the test runs. I used this opportunity to write TestEnvironment helper class to track environmental variants before and after an individual test executes. This class defines a common set of things that should not change (temp path, environment variables, etc.) as a TestVariant and things that are expected to change (environment variables, temp files/folders, etc.) as a TestInvariant which is automatically reverted when the TestEnvironment is disposed. This class is generic and can be extended to additional cases. Related to dotnet#1808
Addresses an issue in VerifyMultipleRequestForSameProjectWithErrors_ErrorAndContinue where the test will fail (writing an MSBuild_*.failure.txt file in the temp folder) but is not detected because a failure was expected. The primary change is to detect the number of MSBuild_*.failure.txt files and assert the value is unchanged before and after the test runs. I used this opportunity to write TestEnvironment helper class to track environmental variants before and after an individual test executes. This class defines a common set of things that should not change (temp path, environment variables, etc.) as a TestVariant and things that are expected to change (environment variables, temp files/folders, etc.) as a TestInvariant which is automatically reverted when the TestEnvironment is disposed. This class is generic and can be extended to additional cases. Related to dotnet#1808
Addresses an issue in VerifyMultipleRequestForSameProjectWithErrors_ErrorAndContinue where the test will fail (writing an MSBuild_*.failure.txt file in the temp folder) but is not detected because a failure was expected. The primary change is to detect the number of MSBuild_*.failure.txt files and assert the value is unchanged before and after the test runs. I used this opportunity to write TestEnvironment helper class to track environmental variants before and after an individual test executes. This class defines a common set of things that should not change (temp path, environment variables, etc.) as a TestVariant and things that are expected to change (environment variables, temp files/folders, etc.) as a TestInvariant which is automatically reverted when the TestEnvironment is disposed. This class is generic and can be extended to additional cases. Related to dotnet#1808
@AndyGerlicher does 1022b48 correct MSBuild hangs in general? Or, is it a targeted fix for hangs during compilation? I ask because we've seen hangs while MSBuild is testing projects and / or without *.failure.txt files. Sticking to the *.failure.txt cases, does 1022b48 avoid both stack traces shown in my comment above? And, which MSBuild release will include 1022b48? |
From @dougbu on February 25, 2017 23:21
Steps to reproduce
dougbu/does.it.hang.yes.it.does
branch.\build.cmd initialize
build.cmd
to compile (this involves wrapper / waitingsake.exe
anddotnet
processes) or run inner MsBuild command with stderr redirected. I.e. execute one of.\build.cmd build-compile
dotnet msbuild /nologo "C:\dd\dnx\Universe\Mvc\.build/makefile.proj" "/p:LifecycleType=Standard;Configuration=Debug" /t:Compile 2>build.err
Have similar issues with
2>&1 |tee build.log
,|tee build.log
or2>&1
in the second case. The unadorned second command and that with2>build.err >build.log
usually work fine.Expected behavior
Build completes and exits cleanly.
Errors and warnings during the build are expected as well, especially MSB3277 due to .microsoft/vstest#393 or perhaps because Mvc.Core project depends on Microsoft.Extensions.DependencyModel version
1.2.0-*
while the xUnit runner uses version1.1.0
.Actual behavior
Commands hang consistently. (Hang may take some work in Task Manager due to dotnet/cli#4856.)
It's not predictable what project is last mentioned in
build.log
. But, hangs seem to occur when the build is near its end.There's never just one
dotnet
process when hangs occur. Usually see the original command, thedotnet exec
command (to invoke the latest copy ofdotnet.exe
), and at least one of the worker processes.Environment data
dotnet --info
output:Additional information
I have process dumps captured in Visual Studio and Task Manager. If they will help, contact me at my Microsoft email (same alias as here) and I'll share.
Problems appear specific to this branch. Command-line builds complete reliably in the
dougbu/migration
branch for example. I cleaned up somedotnet migrate
artifacts in thedougbu/migration
branch and the*.sln
files are different. But, largest change is probably a fix that's only in thedougbu/migration
branch to build justMvc.sln
and not all three solutions.Copied from original issue: dotnet/cli#5849
The text was updated successfully, but these errors were encountered: