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

Writes to the plugins-cache directory are not concurrency-safe #8129

Closed
bergbria opened this issue May 13, 2019 · 3 comments
Closed

Writes to the plugins-cache directory are not concurrency-safe #8129

bergbria opened this issue May 13, 2019 · 3 comments
Labels
Area:Plugin V2 plugin w/ cross platform support Product:NuGet.exe NuGet.exe

Comments

@bergbria
Copy link

Details about Problem

NuGet product used (NuGet.exe | VS UI | Package Manager Console | dotnet.exe):

NuGet version: 4.9.4

Detailed repro steps so we can see the same problem

  1. Find a scenario that requires using the plugins-cache directory
    1. My scenario (issue is not specific to this scenario)
      1. Select an Azure DevOps feed for testing
      2. Install https://github.com/Microsoft/artifacts-credprovider
  2. Run lots of concurrent nuget processes that use that dir
  3. E.g. nuget <install|restore> <various packages found on the devops feed>

Some number of these will fail with errors like the following:
The process cannot access the file 'C:\Users\foo\AppData\Local\NuGet\plugins-cache\9501a58c1599e50ae562d549080e69874f89a068$CredentialProvider.Microsoft.exe\Source-Agnostic.dat-new' because it is being used by another process. Errors in packages.config projects The process cannot access the file 'C:\Users\foo\AppData\Local\NuGet\plugins-cache\9501a58c1599e50ae562d549080e69874f89a068$CredentialProvider.Microsoft.exe\Source-Agnostic.dat-new' because it is being used by another process.

Suspected code

I believe the issue is that PluginCacheEntry.UpdateCacheFileAsync is not concurrency safe. Specifically this block:

if (!File.Exists(CacheFileName))
{
    File.Move(
        NewCacheFileName,
        CacheFileName);
}

Ideally, this method would either use a mutex or simply swallow failures rather than throwing.

@zivkan
Copy link
Member

zivkan commented May 14, 2019

Could you explain why you're running many nuget processes in parallel?

When you restore a solution, NuGet will restore all the projects in the solution, in parallel. If you're using nuget install to get some packages in a directory, you can use packages.config to list all the packages, then restore that file, and again, nuget will restore them in parallel.

I'm not saying that the error you reported isn't an issue, but it sounds like there are things you might be able to change to get a better experience than if we only fixed the bug you reported.

@nkolev92 nkolev92 added the Area:Plugin V2 plugin w/ cross platform support label May 14, 2019
@nkolev92
Copy link
Member

Are all processes sharing the same temp folder?

There should be process-wide locking based on https://github.com/NuGet/NuGet.Client/blob/1bb100cd4df524de75356ccfd146041eb95c2d59/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs#L199-L200.

@bergbria
Copy link
Author

@nkolev92, it looks like different temp directories were indeed the problem. Thanks for suggesting that; I didn't notice it in my skimming of the code.

@zivkan, I'm running many processes in parallel for a variety of reasons, probably mostly unique to the Office build system (the simplest reason is that there isn't a single solution but it rapidly grows more complex). I'd be happy to discuss the details with you in a different forum if you want, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:Plugin V2 plugin w/ cross platform support Product:NuGet.exe NuGet.exe
Projects
None yet
3 participants