Skip to content
This repository has been archived by the owner on Dec 18, 2017. It is now read-only.

Fix concurrency issues around writing files to global locations in kpm #437

Closed
davidfowl opened this issue Jul 13, 2014 · 4 comments
Closed
Assignees
Milestone

Comments

@davidfowl
Copy link
Member

We need to do this for the kpm cache and when writing the nupkg files on disk.

@ChengTian
Copy link
Contributor

@davidfowl , I wrote a program which starts two kpm pack simultaneously and found the following race condition:

When we want to write a nukpg to global cache, we first delete the old one in cache and copy the new nupkg from a temp file to the final destination. See https://github.com/aspnet/KRuntime/blob/dev/src/Microsoft.Framework.PackageManager/Restore/NuGet/HttpSource.cs#L119.

After we write the nupkg to global cache, we also want to extract its content into current project's local package dir, so we need to read it. See https://github.com/aspnet/KRuntime/blob/dev/src/Microsoft.Framework.PackageManager/Restore/NuGet/PackageFeed.cs#L204

So part of the logic in kpm restore can be simplified to:

1. File.Delete("global_cache\A.1.0.0.0.nukpg");
2. File.Copy("tmp.nukpg", "global_cache\A.1.0.0.0.nukpg");
3. ZipArchive.Extract("global_cache\A.1.0.0.0.nukpg", "MyProject\packages\A.1.0.0.0");

When we execute two kpm restore concurrently (p1 and p2), let's assume the two processes interleave in the following way:

p1: 1    2      3
p2:          1

That is, after p2 deletes the nupkg in global cache, p1 tries to read the nukpg and throws an exception because the nukpg is not there.

We can fix this problem by keeping the global nukpg "locked" before it's extracted. I'll send my proposed solution as a PR later.

@ChengTian
Copy link
Contributor

@davidfowl , BTW, you mentioned that you have some further instructions for this issue. Please put them here at your earliest convenience. Thanks!

@davidfowl
Copy link
Member Author

@ChengTian
Copy link
Contributor

@davidfowl , thanks for the useful hints. However, because we are using a lot of async methods, I have trouble using Mutex to resolve race conditions. In the PR below I use Semaphore to eliminate contentions successfully. To learn about the problem caused by mixing Mutex and async methods, please see http://www.dzhang.com/blog/2012/08/29/synchronization-in-async-csharp-methods

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants