The .Net frog

J'ai vu ce genre de choses dans plusieurs bouts de code récemment:

lock(this)
{
    // Do stuff...
}

Même si cela fonctionne, c'est une très mauvaise idée! Vous ne devriez jamais (ou du moins je n'ai aucune bonne raison en tête) faire un lock sur un type public, ce qui inclue évidemment le "this". Il y a une raison très simple à cela: vous n'avez aucun moyen de maitriser ce que le code appelant va essayer de faire.

Prenons un exemple simple. Dans le code suivant, nous avons une class qui utilise un lock sur "this" (On l'appellera ClassThatLocksItself). Nous avons aussi une seconde class (CallerClass) qui va appeler la première. CallerClass va alors utiliser un lock sur l'instance de ClassThatLocksItself lors de cet appel.

    class Program
    {
        static void Main(string[] args)
        {
            ClassThatLocksItself myObj = new ClassThatLocksItself();
 
            CallerClass caller = new CallerClass(myObj);
            caller.LockTheObjectInAThread();
            Thread.Sleep(500);
 
            myObj.LockMe();
        }
    }
 
    class CallerClass
    {
        private ClassThatLocksItself _myObj;
        public CallerClass(ClassThatLocksItself myObj)
        {
            _myObj = myObj;
        }
 
        public void LockTheObjectInAThread()
        {
            ThreadPool.QueueUserWorkItem(LockTheObject);
        }
 
        public void LockTheObject(object state)
        {
            Console.WriteLine("Acquiring lock on the object");
            lock (_myObj)
            {
                Thread.Sleep(100000); // Do a long computation
            }
            Console.WriteLine("Releasing lock on the object");
        }
    }
 
    public class ClassThatLocksItself
    {
        public void LockMe()
        {
            Console.WriteLine("ClassThatLocksItself -- Trying to acquire lock on this");
            lock (this)
            {
                Console.WriteLine("ClassThatLocksItself -- lock on this acquired");
            }
            Console.WriteLine("ClassThatLocksItself -- lock on this released");
        }

Si vous essayez d'exécuter ce code, vous remarquerez que Console.WriteLine("ClassThatLocksItself -- lock on this acquired"); est seulement executé lorsque la fonction LockTheObject() retourne (dans notre cas, après 100 secondes!). En utilisant lock(this), vous êtes devenu dépendant de facteurs externes inconnus (dans notre exemple, CallerClass a décidé d'utiliser l'instance de CLassThatLocksItself pour la synchronisation, ce que ClassThatLocksItself n'avait pas moyen de savoir). La situation est même pire pour les développeurs qui veulent réutiliser votre ClassThatLocksItself: a moi de parcourir la source de la classe, ils n'ont aucun moyen de savoir qu'il peut y avoir de grave problèmes de synchronisation.

Maintenant, essayez de deviner ce qui va se passer si l'on modifie l'implémentation de LockTheObject() par la suivante:

public void LockTheObject(object state)
        {
            Console.WriteLine("Acquiring lock on the object");
            lock (_myObj)
            {
                _myObj.LockMe(); // will not lock!
            }
            Console.WriteLine("Releasing lock on the object");
        }

Un certain nombre d'entre vous devrait parier sur le deadlock je suppose. Mais non! En fait, la CLR est assez intélligente pour détecter que vous avez déjà acquis le lock sur le même objet lorsque LockMe execute lock(this). N'oubliez pas, nous sommes dans le même thread! Cela rend lock(this) très subtile: vous n'aurez des problèmes que dans des cas très spécifiques!.