New inside the castle

I noticed the following code from our foreign programmers:

private Client[] clients = new Client[0];

public CreateClients(int count)
{
    lock (clients)
    {
        clients = new Client[count];

        for(int i=0; i<count; i++)
        {
           Client[i] = new Client();//Stripped
        }
     }
 }

This is not entirely correct code, but I was wondering what exactly this will do. Will this lock a new object every time this method is called?

+5
source share
5 answers

To answer your question, “I was wondering what exactly this will do,” think about what happens if two threads try to do this.

Thread 1: locks on the clients reference, which is `new Client[0]`
   Thread 1 has entered the critical  block
Thread 1: makes a array and assigns it to the clients reference
Thread 2: locks on the clients reference, which is the array just made in thread 1
   Thread 2 has entered the critical block

You know that there are two threads in a critical block at the same time. This is bad.

+5
source

This code is incorrect - it will block a new instance each time it is called.

It should look like this:

private static readonly object clientsLock = new object();
private static string[] Clients = null;

public CreateClients(int count)
{
    if(clients == null)
    {
        lock (clientsLock)
        {
            if(clients == null)
            {
                clients = new string[count];

                for(int i=0; i<count; i++)
                {
                     client[i] = new Client();//Stripped
                }
            }
        }
     }
 }

, , - if.

+1

:
private object = new Object();

lock(object){

//your code

}
+1

lock . , , , , lock . 2 , lock, , , , .

- ,

private readonly object clientsLock = new object();
private Client[] clients = new Client[0];  

public CreateClients(int count) {     
  lock (clientsLock) {         
    clients = new string[count]; 
    ...
  }
}
+1

, !

- , . .

0

All Articles