0

This function takes a few seconds to run locally. It unpacks a zip file onto the local server and is called before serving individual files in the zip. It should only run once if the extracted folder does not exist.

internal static void Init(string releaseName)
{
    var unpackedFolderDirectory = Settings.Editor.FilesRootFolder + releaseName + "\\";
    var dirInfo = new DirectoryInfo(unpackedFolderDirectory);
    if (dirInfo.Exists) return;

    lock ("UnpackLock")
    {
        dirInfo.Refresh();
        if (dirInfo.Exists) return;

        // Unpack all
        var bytes = getBytesFromAzure();
        using var ms = new MemoryStream(bytes);
        using (var archive = new ZipArchive(ms))
        {
            archive.ExtractToDirectory(unpackedFolderDirectory);
        }
    }
}

If I make multiple requests to this function, some of them return the error:

The file 'C:\SomeFolder\SomeSubFolder\SomeFile.png' already exists.
On line -> archive.ExtractToDirectory(unpackedFolderDirectory);

I am expecting the archive.ExtractToDirectory(unpackedFolderDirectory); to only execute once, but it appears to be running multiple times.

What am I doing wrong? Is there some race condition here I'm not spotting?

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
Tom Gullen
  • 61,249
  • 84
  • 283
  • 456
  • 1
    Not sure if locking on a string is a good idea. – Fildor Apr 08 '22 at 14:51
  • 1
    @Fildor thank you! Locking on a static object fixes the issue. It would be great to understand why the code I posted doesn't work. Is it because the string instance is non static? – Tom Gullen Apr 08 '22 at 14:53
  • 1
    It basically comes down to how strings work in C#/.Net. All the specialities with interning, caching etc. So, while it's the same string all the time, it doesn't mean that for the system it's the identical "object" / reference. You can even end up clogging other parts of the software if you were to use the same string to lock elsewhere. – Fildor Apr 08 '22 at 15:02
  • 1
    See also: https://jonskeet.uk/csharp/threads/lockchoice.html Paragraph 4 : _"One other practice occasionally seen is that of locking on string literals, for example declaring string someLock = "Lock guarding some data"; and then locking on someLock. This has the same problem as the other locking strategies, due to string interning - if two classes both use the same string literal, they'll end up using references to the same string, so they'll be sharing a lock without realising it. If you really want to make a lock with a description, you can always create your own Lock class. ... "_ – Fildor Apr 08 '22 at 15:06
  • Related: [Is it OK to use a string as a lock object?](https://stackoverflow.com/questions/12804879/is-it-ok-to-use-a-string-as-a-lock-object) / [Why only literal strings saved in the intern pool by default?](https://stackoverflow.com/questions/8509035/why-only-literal-strings-saved-in-the-intern-pool-by-default) These don't explain why the `lock ("UnpackLock")` doesn't work in your case though. – Theodor Zoulias Apr 08 '22 at 16:22

0 Answers0