New inside a lock

Question

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
        }
     }
 }

It's not exactly proper code but I was wondering what exactly this will do. Will this lock on a new object each time this method is called?

Answer 1

To answer your question of "I was wondering what exactly this will do" consider 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 have two threads in the critical block at the same time. That's bad.

Answer 2

This lock really does nothing. It locks an instance of an object which is immediately changed such that other threads entering this method will lock on a different object. The result is 2 threads executing in the middle of the lock which is probably not what was intended.

A much better approach here is to use a different, non-changing object to lock on

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

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