¿Por qué lock (this) {}} es malo?

Porque si las personas pueden acceder a su instancia de objeto (es decir, su puntero this), también pueden intentar bloquear ese mismo objeto. Ahora pueden no ser conscientes de que estás bloqueando this internamente, por lo que esto puede causar problemas (posiblemente un punto muerto)

Además de esto, también es una mala práctica, porque está bloqueando "demasiado"

Por ejemplo, es posible que tenga una variable miembro de List<int>, y lo único que realmente necesita bloquear es esa variable miembro. Si bloquea todo el objeto en sus funciones, entonces otras cosas que llaman a esas funciones se bloquearán esperando el bloqueo. Si esas funciones no necesitan acceder a la lista de miembros, estarás haciendo que otro código espere y ralentice tu aplicación sin ningún motivo.

Echa un vistazo al tema de MSDN Sincronización de subprocesos (Guía de programación en C#)

En general, es mejor evitar el bloqueo en un tipo público o en un objeto instancias fuera del control de su aplicación. Por ejemplo, bloquear (esto) puede ser problemático si la instancia puede acceso público, porque el código más allá de su control puede bloquear el objeta también. Esto podría crear situaciones de punto muerto en las que dos o más hilos de espera para el lanzamiento de la mismo objeto. Bloqueo de un público tipo de datos, a diferencia de un objeto, puede causar problemas para el mismo motivo. Bloquear cadenas literales es especialmente arriesgado porque literal las cuerdas son internadas por el común language runtime (CLR). Esto significa que hay una instancia de cualquier dado literal de cadena para la totalidad programa, el mismo objeto exacto representa el literal en toda la ejecución dominios de aplicación, en todos los hilos. Como resultado, un bloqueo colocado en una cadena con el mismo contenido en cualquier lugar de la proceso de aplicación bloquea todo instancias de esa cadena en el aplicación. Como resultado, es mejor para bloquear a un miembro privado o protegido eso no está internado. Algunas clases proporcionar a los miembros específicamente para bloqueo. El tipo de matriz, por ejemplo, proporciona SyncRoot. Muchos colección los tipos proporcionan un miembro SyncRoot como bien.

Sé que este es un hilo viejo, pero debido a que la gente todavía puede buscar esto y confiar en él, parece importante señalar que lock(typeof(SomeObject)) es significativamente peor que lock(this). Dicho esto; felicitaciones sinceras a Alan por señalar que lock(typeof(SomeObject)) es una mala práctica.

Una instancia de System.Type es uno de los objetos más genéricos y de grano grueso que hay. Por lo menos, una instancia de Sistema.Type es global a un AppDomain, y. NET puede ejecutar varios programas en un AppDomain. Esto significa que dos programas completamente diferentes podrían potencialmente causar interferencia entre sí, incluso hasta el punto de crear un punto muerto si ambos intentan obtener un bloqueo de sincronización en la misma instancia de tipo.

Así que lock(this) no es una forma particularmente robusta, puede causar problemas y siempre debe levantar las cejas por todas las razones citadas. Sin embargo, hay un código ampliamente utilizado, relativamente respetado y aparentemente estable como log4net que utiliza el patrón de bloqueo(este) ampliamente, a pesar de que personalmente prefiero ver ese cambio de patrón.

Pero lock(typeof(SomeObject)) abre una nueva y mejorada lata de gusanos.

Por lo que vale.

...y los mismos argumentos se aplican a esta construcción también:

lock(typeof(SomeObject))

Imagine que tiene una secretaria experta en su oficina que es un recurso compartido en el departamento. De vez en cuando, te precipitas hacia ellos porque tienes una tarea, solo para esperar que otro de tus compañeros de trabajo no los haya reclamado ya. Por lo general, solo tiene que esperar un breve período de tiempo.

Debido a que cuidar es compartir, su gerente decide que los clientes también pueden usar la secretaria directamente. Pero esto tiene un efecto secundario: un cliente puede incluso reclamarlos mientras estás trabajando para este cliente y también los necesita para ejecutar parte de las tareas. Se produce un punto muerto, porque reclamar ya no es una jerarquía. Esto podría haberse evitado al no permitir a los clientes reclamarlos en primer lugar.

lock(this) es malo como hemos visto. Un objeto externo puede bloquearlo y, como no controlas quién está usando la clase, cualquiera puede bloquearlo... Que es el ejemplo exacto como se describe anteriormente. Una vez más, la solución es limitar la exposición de la objeto. Sin embargo, si usted tiene un private, protected o internal clase usted ya podría controlar quién está bloqueando su objeto, porque está seguro de que ha escrito su código usted mismo. Así que el mensaje aquí es: no exponerlo como public. Además, asegurarse de que un bloqueo se utiliza en escenarios similares evita bloqueos.

Todo lo contrario de esto es bloquear los recursos que se comparten en todo el dominio de la aplicación the el peor de los casos. Es como poner a tu secretaria afuera y permitiendo que todo el mundo los reclame. El resultado es un caos total - o en términos de código fuente: fue una mala idea; tirarlo y empezar de nuevo. Entonces, ¿cómo lo hacemos?

Los tipos se comparten en el dominio de la aplicación como la mayoría de las personas aquí señalan. Pero hay cosas aún mejores que podemos usar: cuerdas. La razón es que las cadenas se agrupan. En otras palabras: si tiene dos cadenas que tienen el mismo contenido en un dominio de aplicación, existe la posibilidad de que tengan exactamente el mismo puntero. Dado que el puntero se utiliza como la tecla de bloqueo, lo que básicamente obtiene es un sinónimo de "prepararse para un comportamiento indefinido".

Del mismo modo, no debe bloquear objetos WCF, HttpContext.Corriente, Hilo.Corriente, Singletons (en general), etc. La forma más fácil de evitar todo esto? private [static] object myLock = new object();

Bloqueo en el este punteropuede ser malosi está bloqueando un recurso compartido . Un recurso compartido puede ser una variable estática o un archivo en su computadora, es decir, algo que se comparte entre todos los usuarios de la clase. La razón es que este puntero contendrá una referencia diferente a una ubicación en la memoria cada vez que se instancie su clase. Por lo tanto, bloquear sobre this en una instancia de una clase es diferente a bloquear sobre this en otra instancia de una clase.

Echa un vistazo a este código para ver lo que quiero decir. Agregue el siguiente código a su programa principal en una aplicación de consola:

    static void Main(string[] args)
    {
         TestThreading();
         Console.ReadLine();
    }

    public static void TestThreading()
    {
        Random rand = new Random();
        Thread[] threads = new Thread[10];
        TestLock.balance = 100000;
        for (int i = 0; i < 10; i++)
        {
            TestLock tl = new TestLock();
            Thread t = new Thread(new ThreadStart(tl.WithdrawAmount));
            threads[i] = t;
        }
        for (int i = 0; i < 10; i++)
        {
            threads[i].Start();
        }
        Console.Read();
    }

Cree una nueva clase como la siguiente.

 class TestLock
{
    public static int balance { get; set; }
    public static readonly Object myLock = new Object();

    public void Withdraw(int amount)
    {
      // Try both locks to see what I mean
      //             lock (this)
       lock (myLock)
        {
            Random rand = new Random();
            if (balance >= amount)
            {
                Console.WriteLine("Balance before Withdrawal :  " + balance);
                Console.WriteLine("Withdraw        : -" + amount);
                balance = balance - amount;
                Console.WriteLine("Balance after Withdrawal  :  " + balance);
            }
            else
            {
                Console.WriteLine("Can't process your transaction, current balance is :  " + balance + " and you tried to withdraw " + amount);
            }
        }

    }
    public void WithdrawAmount()
    {
        Random rand = new Random();
        Withdraw(rand.Next(1, 100) * 100);
    }
}

Aquí hay una ejecución del programa bloqueando esto.

   Balance before Withdrawal :  100000
    Withdraw        : -5600
    Balance after Withdrawal  :  94400
    Balance before Withdrawal :  100000
    Balance before Withdrawal :  100000
    Withdraw        : -5600
    Balance after Withdrawal  :  88800
    Withdraw        : -5600
    Balance after Withdrawal  :  83200
    Balance before Withdrawal :  83200
    Withdraw        : -9100
    Balance after Withdrawal  :  74100
    Balance before Withdrawal :  74100
    Withdraw        : -9100
    Balance before Withdrawal :  74100
    Withdraw        : -9100
    Balance after Withdrawal  :  55900
    Balance after Withdrawal  :  65000
    Balance before Withdrawal :  55900
    Withdraw        : -9100
    Balance after Withdrawal  :  46800
    Balance before Withdrawal :  46800
    Withdraw        : -2800
    Balance after Withdrawal  :  44000
    Balance before Withdrawal :  44000
    Withdraw        : -2800
    Balance after Withdrawal  :  41200
    Balance before Withdrawal :  44000
    Withdraw        : -2800
    Balance after Withdrawal  :  38400

Aquí hay una ejecución del programa bloqueando myLock.

Balance before Withdrawal :  100000
Withdraw        : -6600
Balance after Withdrawal  :  93400
Balance before Withdrawal :  93400
Withdraw        : -6600
Balance after Withdrawal  :  86800
Balance before Withdrawal :  86800
Withdraw        : -200
Balance after Withdrawal  :  86600
Balance before Withdrawal :  86600
Withdraw        : -8500
Balance after Withdrawal  :  78100
Balance before Withdrawal :  78100
Withdraw        : -8500
Balance after Withdrawal  :  69600
Balance before Withdrawal :  69600
Withdraw        : -8500
Balance after Withdrawal  :  61100
Balance before Withdrawal :  61100
Withdraw        : -2200
Balance after Withdrawal  :  58900
Balance before Withdrawal :  58900
Withdraw        : -2200
Balance after Withdrawal  :  56700
Balance before Withdrawal :  56700
Withdraw        : -2200
Balance after Withdrawal  :  54500
Balance before Withdrawal :  54500
Withdraw        : -500
Balance after Withdrawal  :  54000

Hay muy buen artículo al respecto http://bytes.com/topic/c-sharp/answers/249277-dont-lock-type-objects por Rico Mariani, arquitecto de rendimiento para Microsoft®. NET runtime

Extracto:

El problema básico aquí es que no posee el objeto type, y no sé quién más podría acceder a él. En general, es una muy mala idea confiar en bloquear un objeto que no creaste y no sabes quién más podría estar accediendo. Hacerlo invita a un punto muerto. La forma más segura es solo bloquea objetos privados.

También hay una buena discusión sobre esto aquí: ¿Es este el uso adecuado de un mutex?

Porque cualquier fragmento de código que pueda ver la instancia de tu clase también puede bloquear esa referencia. Desea ocultar (encapsular) su objeto de bloqueo para que solo el código que necesita hacer referencia a él pueda hacer referencia a él. La palabra clave esto se refiere a la instancia de clase actual, por lo que cualquier número de cosas podría tener referencia a ella y podría usarla para hacer sincronización de subprocesos.

Para ser claros, esto es malo porque algún otro fragmento de código podría usar la instancia de clase para bloquear, y podría evitar su código de obtener un bloqueo oportuno o podría crear otros problemas de sincronización de hilo. En el mejor de los casos: nada más usa una referencia a tu clase para bloquear. Caso medio: algo usa una referencia a tu clase para hacer bloqueos y causa problemas de rendimiento. En el peor de los casos: algo usa una referencia de tu clase para hacer bloqueos y causa problemas muy malos, muy sutiles, muy difíciles de depurar.

Aquí hay un código de ejemplo que es más fácil de seguir (IMO): (Funcionará en LINQPad, haciendo referencia a los siguientes espacios de nombres: System.Net y el Sistema.Enhebrando.Tareas)

void Main()
{
    ClassTest test = new ClassTest();
    lock(test)
    {
        Parallel.Invoke (
            () => test.DoWorkUsingThisLock(1),
            () => test.DoWorkUsingThisLock(2)
        );
    }
}

public class ClassTest
{
    public void DoWorkUsingThisLock(int i)
    {
        Console.WriteLine("Before ClassTest.DoWorkUsingThisLock " + i);
        lock(this)
        {
            Console.WriteLine("ClassTest.DoWorkUsingThisLock " + i);
            Thread.Sleep(1000);
        }
        Console.WriteLine("ClassTest.DoWorkUsingThisLock Done " + i);
    }
}

Por favor, consulte el siguiente enlace que explica por qué bloquear (esto) no es una buena idea.

Http://blogs.msdn.com/b/bclteam/archive/2004/01/20/60719.aspx

Así que la solución es agregar un objeto privado, por ejemplo, lockObject a la clase y colocar la región de código dentro de la instrucción lock como se muestra a continuación:

lock (lockObject)
{
...
}

Aquí hay una ilustración mucho más simple (tomada de Pregunta 34 aquí) por qué bloquear(esto) es malo y puede resultar en bloqueos cuando el consumidor de su clase también intenta bloquear el objeto. A continuación, solo uno de los tres hilos puede proceder, los otros dos están bloqueados.

class SomeClass
{
    public void SomeMethod(int id)
    {
        **lock(this)**
        {
            while(true)
            {
                Console.WriteLine("SomeClass.SomeMethod #" + id);
            }
        }
    }
}

class Program
{
    static void Main(string[] args)
    {
        SomeClass o = new SomeClass();

        lock(o)
        {
            for (int threadId = 0; threadId < 3; threadId++)
            {
                Thread t = new Thread(() => {
                    o.SomeMethod(threadId);
                        });
                t.Start();
            }

            Console.WriteLine();
        }

Para trabajar, este tipo usó Hilo.TryMonitor (con tiempo de espera) en lugar de lock:

            Monitor.TryEnter(temp, millisecondsTimeout, ref lockWasTaken);
            if (lockWasTaken)
            {
                doAction();
            }
            else
            {
                throw new Exception("Could not get lock");
            }

Https://blogs.appbeat.io/post/c-how-to-lock-without-deadlocks

Lo siento chicos, pero no puedo estar de acuerdo con el argumento de que bloquear esto podría causar un punto muerto. Estás confundiendo dos cosas: estancado y hambriento.

• No se puede cancelar el bloqueo sin interrumpir uno de los hilos, por lo que después de entrar en un bloqueo no se puede salir
• El hambre terminará automáticamente después de que uno de los hilos termine su trabajo

Aquí hay una imagen que ilustra la diferencia.

Conclusión
Puedes todavía utilice con seguridad lock(this) si la inanición de hilo no es un problema para usted. Todavía tienes que tener en cuenta que cuando el hilo, que está muriendo de hambre usando lock(this) termina en un candado con tu objeto bloqueado, finalmente terminará en hambre eterna;)

Habrá un problema si se puede acceder a la instancia públicamente porque podría haber otras solicitudes que podrían estar usando la misma instancia de objeto. Es mejor usar variable privada / estática.