dotnet / core

.NET news, announcements, release notes, and more!
https://dot.net
MIT License
20.96k stars 4.91k forks source link

Update 3.4.1 breaks building Apache Thrift (permission issue?) on Ubuntu 18.04 #4719

Closed emmenlau closed 4 years ago

emmenlau commented 4 years ago

I've been using dotnet on Ubuntu Linux 18.04 x86_64 (installed as in https://docs.microsoft.com/en-us/dotnet/core/install/linux-package-manager-ubuntu-1804) for almost a year successfully to build https://github.com/apache/thrift. Yesterday I've installed system updates (which may have included an update of dotnet). Usually I install updates at least once every one or two months.

Since today, the build of Thrift fails with error:

[...]
cd /home/user/thrift/lib/netstd/Thrift && /usr/bin/dotnet msbuild Thrift.csproj /nologo /restore /target:Rebuild /property:Configuration=Debug /property:OutDir=/home/user/Debug/thrift/lib/netstd/Thrift/bin /property:BaseIntermediateOutputPath=/home/user/Debug/thrift/lib/netstd/Thrift/obj/
FAILED: lib/netstd/Thrift/bin/Thrift.dll 
cd /home/user/thrift/lib/netstd/Thrift && /usr/bin/dotnet msbuild Thrift.csproj /nologo /restore /target:Rebuild /property:Configuration=Debug /property:OutDir=/home/user/Debug/thrift/lib/netstd/Thrift/bin /property:BaseIntermediateOutputPath=/home/user/Debug/thrift/lib/netstd/Thrift/obj/
MSBUILD : error MSB1025: An internal failure occurred while running MSBuild.
System.ComponentModel.Win32Exception (13): Permission denied
   at System.Diagnostics.Process.set_PriorityClassCore(ProcessPriorityClass value)
   at System.Diagnostics.Process.set_PriorityClass(ProcessPriorityClass value)
   at Microsoft.Build.CommandLine.MSBuildApp.Execute(String[] commandLine)
Unhandled exception. System.ComponentModel.Win32Exception (13): Permission denied
   at System.Diagnostics.Process.set_PriorityClassCore(ProcessPriorityClass value)
   at System.Diagnostics.Process.set_PriorityClass(ProcessPriorityClass value)
   at Microsoft.Build.CommandLine.MSBuildApp.Execute(String[] commandLine)
   at Microsoft.Build.CommandLine.MSBuildApp.Main(String[] args)

I've tried to isolate the cause and could find so far:

This is an acceptable workaround for me, but sooner or later a fix would be appreciated.

dagood commented 4 years ago

Based on the stack trace, I wonder if you're hitting problems with https://github.com/microsoft/msbuild/pull/4162, where you don't have enough permissions to change process priority?

It sounds like you upgraded from SDK 3.1.201 to 3.1.300, which looks like it includes that MSBuild update.

emmenlau commented 4 years ago

Thanks @dagood for the quick reply. It seems possible, but I fail to see all the details of microsoft/msbuild#4162.

Am I correct to understand that it tries to set a lower process priority, what on Linux is represented with a higher integer number? I tested and my permissions allow to set a lower priority (this is typically allowed for all users), so i.e. the following works:

#> nice -n10 echo hallo
hallo

Whereas I can not set a higher priority (this is typically forbidden), so i.e. the following fails:

#> nice -n-10 echo hallo
nice: cannot set niceness: Permission denied

One more thing that might be related: My default priority is not zero (as one would typically assume on Linux). I've set it slightly lower to 5 which gives better overall user experience on a multi-user machine.

dagood commented 4 years ago

That stack trace goes (well, appears to go) here in particular:

https://github.com/microsoft/msbuild/blob/7869acfe0be8e983da5de514b896cca6f38fd645/src/MSBuild/XMake.cs#L628-L631

ProcessPriorityClass priority = lowPriority ? ProcessPriorityClass.BelowNormal : ProcessPriorityClass.Normal;
Process.GetCurrentProcess().PriorityClass = priority;

The priority values per enum value seem to be here (not sure where in docs):

https://github.com/dotnet/runtime/blob/4f9ae42d861fcb4be2fcd5d3d55d5f227d30e723/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetSetPriority.cs#L57-L58

So it does appear to be relying on that exact assumption you're talking about for default priority. As far as I can tell (I'm not familiar with this change, just found this issue interesting) it's trying to raise it back to "normal" 0. I'd suggest raising this issue in the MSBuild repo.

dagood commented 4 years ago

By the way, as far as a workaround goes, on https://dotnet.microsoft.com/download/dotnet-core/3.1 you can see there are actually several SDK versions released for v3.1.4: 3.1.300, 3.1.202, and 3.1.104. I'd expect 3.1.202 to not have this MSBuild change, yet still have the 3.1.4 .NET Core Runtime. You'd still have to pin the version since our package names only have "3.1" granularity, but you can at least get the 3.1.4 security patch.

tianon commented 4 years ago

I'm seeing this same issue while using Docker on my development system where I set all my containers to be set to nice level 19 (which keeps my system more responsive in the face of a misbehaving container). I can also reproduce on a coworker's machine with a nice level of 9.

$ dotnet restore foo/bar.sln
MSBUILD : error MSB1025: An internal failure occurred while running MSBuild.
System.ComponentModel.Win32Exception (13): Permission denied
   at System.Diagnostics.Process.set_PriorityClassCore(ProcessPriorityClass value)
   at System.Diagnostics.Process.set_PriorityClass(ProcessPriorityClass value)
   at Microsoft.Build.CommandLine.MSBuildApp.Execute(String[] commandLine)
Unhandled exception. System.ComponentModel.Win32Exception (13): Permission denied
   at System.Diagnostics.Process.set_PriorityClassCore(ProcessPriorityClass value)
   at System.Diagnostics.Process.set_PriorityClass(ProcessPriorityClass value)
   at Microsoft.Build.CommandLine.MSBuildApp.Execute(String[] commandLine)
   at Microsoft.Build.CommandLine.MSBuildApp.Main(String[] args)

When I set my nice level to 0, it works.

I see there was an attempt to fix this in https://github.com/dotnet/msbuild/pull/5381, but the "fix" was a != on the current priority, so it still attempts to raise the priority if the process priority is below the desired value but isn't exactly the "idle" value, discussed in https://github.com/dotnet/msbuild/pull/5381#discussion_r431447074... So it sounds like the translation from the Linux/POSIX standard nice values into the .NET specific ProcessPriorityClass values is the reason this can't be fixed cleanly?

Alternatively, perhaps the solution is to capture the "permission denied" error and quietly ignore it, since in this case, failure to adjust the process priority is harmless?

What is the value of Process.GetCurrentProcess().PriorityClass if the process's current nice value doesn't map precisely to one of the predefined values from the enum? I guess that's what https://github.com/dotnet/runtime/blob/0bed2de8ed48e7d8ccb0bb7e6c878e22d0b2fa14/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetSetPriority.cs#L38-L61 is? Perhaps a function could be added to allow comparing ProcessPriorityClass values similar to those comparison chains?

tianon commented 4 years ago

To be more specific, the fix in https://github.com/dotnet/msbuild/pull/5381 will probably work on my system which is set to 19, but will not work on the OP's system set to 5 nor will it work on my coworker's system set to 9 (because those values don't map to "idle").

dagood commented 4 years ago

I think you should comment on https://github.com/dotnet/msbuild/issues/5365 or--probably even better--raise a new issue in that repo. I don't think anyone there is aware of this dotnet/core repo issue. I should have closed this as a dupe of https://github.com/dotnet/msbuild/issues/5365 when that was filed to make that clearer, sorry I missed that.

Yeah, the priority API appears to be designed for Windows (historically makes sense) and that makes this awkward... not sure what the best approach is. It seems weird to me that MSBuild is checking/setting priority at all when it's not explicitly passed a parameter telling it to do so, but that's my personal expectation there.

mihsoft commented 4 years ago

Same error here on Ubuntu1804

dagood commented 4 years ago

Closing in favor of discussion in https://github.com/dotnet/msbuild/pull/5381 and https://github.com/dotnet/msbuild/issues/5365.