0

everyone!

I do a small project for my company and I use C#. I have a script for my project. But before this day, my colleagues and I had an idea that the script would be used by users one by one. For example, if there are a user A and user B, there can be the order where the user B runs the script and only then the user A can run the script.

Today the decision was made to give the users the possibility to run the script freely without the predetermined order. And now I have some thoughts. Here the part of the script:

        if (Directory.Exists(@"H:\" + doc_number + @"\detached") == false)
        {
            Directory.CreateDirectory(@"H:\" + doc_number + @"\detached");
            File.WriteAllBytes(@"H:\" + doc_number + @"\detached\1.cms", signature_bytes);
        }
        else
        {
            string[] files = Directory.GetFiles(@"H:\" + doc_number + @"\detached"); int files_number = files.Length;
            File.WriteAllBytes(@"H:\" + doc_number + @"\detached\" + Convert.ToString(files_number + 1) + ".cms", signature_bytes);
        }

Firstly, there is a check of the existence of a directory. If it doesn't exist, the directory will be created and the first file will be added there. Otherwise, we just count the number of files in the directory and then create a new file with a name which is the number of the files in the folder plus one.

However, I'm thinking about the situation when the user A and the user B were at the beginning of this part of the script at the same time and the condition for both would be positive so it wouldn't be executed correctly. Or if one of them started running this part earlier but his or her PC was less powerful so while creating the directory another user would go through the condition, counting files and start creating a file before the first user which would be also incorrect.

I don't know how likely one of these situations are. if so, how can I solve it?

  • It is a possibility, but an extremely unlikely one. In order for that to happen, users would have to execute this code probably within 1/10th of a second of each other. But here's the thing: it really doesn't matter. `Directory.CreateDirectory` doesn't throw an exception if the directory already exists, it just doesn't do anything. The only potentially problematic line is the second line in your `if` block, but you can just get rid of it because the code in your `else` block will work for both scenarios. – Jesse Feb 17 '23 at 16:59
  • If it can happen, you must assume it will and write appropriately safe code. – Eric J. Feb 17 '23 at 17:00
  • Also side note, please don't put multiple expressions on the same line. It makes your code much harder to read. Never sacrifice readability for less lines. It's never worth it. – Jesse Feb 17 '23 at 17:00
  • The numbering matters? Do you just want to allow both users to create files in the same location at the same time? Does it has to be numbers? – Magnetron Feb 17 '23 at 17:41

2 Answers2

1

Indeed, you can run into concurrency issues. And you are correct that you can't rely on the existence of a directory to decide what branch to take in your if statement because you might have operations execute in this order:

User A: Checks for directory. Does not exist. 
User B: Checks for directory. Does not exist. 
User A: Creates directory, enters if branch. 
User B: Creates directory, enters if branch.

If the code was running in one process on one machine but in multiple threads, you could use a lock statement.

If the code was running on different processes on the same machine, you could use a cross-process coordination method such as a Mutex.

The question implies that the code runs on different computers but accesses the same file system. In this case, a lock file is a common mechanism to coordinate access to a shared resource. In this approach, you would attempt to create a file and lock it. If that file already exists and is locked by another process, you know someone else got there first. Depending on your needs, a common scenario is to wait for the lock on the file to go away then acquire the lock yourself and continue.

This strategy also works for the other 2 cases above, though is less efficient.

For information about how to create a file with a lock, see

How to lock a file with C#?

Eric J.
  • 147,927
  • 63
  • 340
  • 553
1

There are some issues with your code. For example, what would happen if a file is deleted? The number of files in the directory would be different than the number of the last file, and you can end up trying to write a file that already exists. Also, please use Path.Combine to create paths, it is safer. You also don't need to check if the directory exists, since Directory.Create will do nothing if it already exists.

Common for all solutions bellow:

string baseDir = Path.Combine("H:",doc_number, "detached");
Directory.Create(baseDir);

If you just want any number of users to create files in the same directory, some solutions that are more safe:

  • Use a GUID:
var guid = Guid.NewGuid();
var file = Path.Combine(baseDir, $"{guid}.cms");
File.WriteAllBytes(file, signature_bytes);
  • Iterate, trying to create a new file:
bool created = false;
int index = 1;
while(!created)
{
    //Check first if the file exists, and gets the next available index
    var file = Path.Combine(baseDir, $"{index}.cms");
    while(File.Exists(file))
    {
         file = Path.Combine(baseDir, $"{++index}.cms");
    }
    //Handle race conditions, if the file was created after we checked
    try
    {        
        //Try create the file, not allowing others to acess it while open
        using var stream = File.Open(file,FileMode.CreateNew,FileAccess.Write,FileShare.None);
        stream.Write(signature_bytes);
        created = true;
    }
    catch (IOException) //If the file already exists, try the next index
    {
        ++index;
    }
}
Magnetron
  • 7,495
  • 1
  • 25
  • 41