Ostatnio przeglądając kod udało mi się znaleźć następujący fragment:

lock (this)
{
  // Do something
}

Do zsynchronizowania dostępu do współdzielonego zasobu używany jest obiekt, w którym występuje ten fragment kodu. Teoretycznie fragment ten jest poprawny. Co więcej w programie nie wystąpił żaden problem z zakleszczeniem.

Przeglądając literaturę możemy natrafić na następujące przykłady:

1. W O’Reilly Programming C# można spotkać następujący przykład użycia słowa kluczowego lock:

public void Incrementer()
{
  try
  {
    while (counter < 1000)
    {
      int temp;
      lock (this)
        {
          temp = counter;
          temp++;
          Thread.Sleep(1);
          counter = temp;
        }

        Console.WriteLine(“{0}. Incrementer: {1}”, Thread.CurrentThread.Name, temp);
    }
  }
}&#91;/code&#93;
<p style="text-align: justify;">Oprócz powyższego przykładu w książce nie ma dyskusji nad jego wadami i zaletami.</p>
<p style="text-align: justify;">2. Andrew Troelsen w Pro C# 2008 and the .NET 3.5 Platform zaleca użycie <em>lock (this)</em> w przypadku metod prywatnych:</p>
[code lang="csharp"]private void SomePrivateMethod()
{
  lock (this)
  {
    // do something
  }
}

W obu przypadkach przemilczane jest zagrożenie, które niesie z sobą wykorzystanielock (this). Przeanalizujmy poniższy kod:

public class BadLock
{
  public void Compute()
  {
    Console.WriteLine("Compute - Start");
    lock (this)
    {
      Console.WriteLine("Compute - In Lock");
      Thread.Sleep(5000);
    }

    Console.WriteLine("Compute - Stop");
  }
}

Klasa BadLock posiada w sobie metodę Compute, które symuluje proces obliczania oraz używa do synchronizacji lock (this). W większości przypadków wykorzystanie tego kodu nie powinno spowodować problemów. Istnieje jednak scenariusz, w którym może dojść do zakleszczenia. W sytuacji, gdy stworzymy obiekt typu BadLock i użyjemy jego do zapewnienia synchronizacji, a następnie wykonamy jakąś metodę z klasy BadLock może dojść do zakleszczenia. Przykład takiego scenariusza przedstawia poniższy kod:

public static void BadLockTest()
{
  Console.WriteLine("Bad Lock test 1");
  BadLock badLock = new BadLock();
  new Thread(new ParameterizedThreadStart(BackgroundMethod)).Start(badLock);
  Thread.Sleep(500);
  badLock.Compute();
}

private static void BackgroundMethod(Object lockable)
{
  lock (lockable)
  {
    Console.Out.WriteLine("BackgroundMethod - Lock");
    Thread.Sleep(Timeout.Infinite);
  }
}

Po uruchomieniu aplikacji otrzymamy następujący wynik:

Bad Lock test 1
BackgroundMethod - Lock
Compute – Start

Nie ważne jak długo będzie się czekać, dalsze komunikaty nie zostaną wyświetlone.

Rozwiązaniem tego problemu jest zastąpienie konstrukcji:

lock (this)
{
  ...
}

następującą konstrukcją:

private object synchronizationObject = new Object();
...
lock (synchronizationObject)
{
  ...
}

Dopiero po zastosowaniu tej konstrukcji mamy pewność, że nikt nie popsuje stanu obiektu i nie doprowadzi do zakleszczenia. Po wprowadzeniu zmian obiekt BadLock będzie wyglądał następująco:

public class GoodLock
{
  private readonly Object synchronizationObject = new Object();

  public void Compute()
  {
    Console.WriteLine("Compute - Start");
    lock (synchronizationObject)
    {
      Console.WriteLine("Compute - In Lock");
      Thread.Sleep(5000);
    }

    Console.WriteLine("Compute - Stop");
  }
}

i otrzymamy poprawne wyniki:

Good Lock test 1
BackgroundMethod - Lock
Compute - Start
Compute - In Lock
Compute – Stop

Na koniec pozostaje jeszcze temat związany z metodą BackgroundMethod. Nie jestem zwolennikiem programowania defensywnego, ale w tym przypadku zamiast ustawiać lock na obiekcie, z którego korzystamy proponowałbym również stworzyć niezależny obiekt służący do synchronizacji.

Check it:

Default ThumbnailEnum i domyślna inicjalizacja wartości