¿Por qué el bloqueo (esto) {...} es malo?

Question

El MSDN documentation dice que

public class SomeObject 
{ 
    public void SomeOperation() 
    { 
    lock(this) 
    { 
     //Access instance variables 
    } 
    } 
} 

es "un problema si la instancia se puede acceder públicamente". Me pregunto por qué? ¿Es porque la cerradura se mantendrá más tiempo de lo necesario? ¿O hay alguna razón más insidiosa?

Answer 1

Es mala forma de utilizar this en los estados de bloqueo, ya que es generalmente fuera de su control, que otra cosa podría estar bloqueando en ese objeto.

Para planificar correctamente las operaciones paralelas, se debe tener especial cuidado para considerar posibles situaciones de interbloqueo, y tener un número desconocido de puntos de entrada de bloqueo lo impide. Por ejemplo, cualquiera con una referencia al objeto puede bloquearlo sin que el diseñador/creador del objeto lo sepa. Esto aumenta la complejidad de las soluciones de subprocesos múltiples y puede afectar su corrección.

Un campo privado suele ser una mejor opción ya que el compilador impondrá restricciones de acceso a él, y encapsulará el mecanismo de bloqueo. El uso de this viola la encapsulación al exponer al público parte de su implementación de bloqueo. Tampoco está claro que va a adquirir un bloqueo en this a menos que haya sido documentado. Incluso entonces, depender de la documentación para evitar un problema no es óptimo.

Finalmente, existe la idea errónea de que lock(this) realmente modifica el objeto pasado como parámetro, y de alguna manera lo hace de solo lectura o inaccesible. Esto es falso. El objeto pasado como parámetro a lock simplemente sirve como una clave . Si ya se mantiene un bloqueo en esa tecla, no se puede hacer el bloqueo; de lo contrario, el bloqueo está permitido.

Es por eso que es malo utilizar cadenas como las claves en las sentencias lock, ya que son inmutables y se comparten/se pueden acceder a través de partes de la aplicación. Debería usar una variable privada en su lugar, una instancia de Object lo hará muy bien.

Ejecute el siguiente código C# como ejemplo.

public class Person 
{ 
    public int Age { get; set; } 
    public string Name { get; set; } 

    public void LockThis() 
    { 
     lock (this) 
     { 
      System.Threading.Thread.Sleep(10000); 
     } 
    } 
} 

class Program 
{ 
    static void Main(string[] args) 
    { 
     var nancy = new Person {Name = "Nancy Drew", Age = 15}; 
     var a = new Thread(nancy.LockThis); 
     a.Start(); 
     var b = new Thread(Timewarp); 
     b.Start(nancy); 
     Thread.Sleep(10); 
     var anotherNancy = new Person { Name = "Nancy Drew", Age = 50 }; 
     var c = new Thread(NameChange); 
     c.Start(anotherNancy); 
     a.Join(); 
     Console.ReadLine(); 
    } 

    static void Timewarp(object subject) 
    { 
     var person = subject as Person; 
     if (person == null) throw new ArgumentNullException("subject"); 
     // A lock does not make the object read-only. 
     lock (person.Name) 
     { 
      while (person.Age <= 23) 
      { 
       // There will be a lock on 'person' due to the LockThis method running in another thread 
       if (Monitor.TryEnter(person, 10) == false) 
       { 
        Console.WriteLine("'this' person is locked!"); 
       } 
       else Monitor.Exit(person); 
       person.Age++; 
       if(person.Age == 18) 
       { 
        // Changing the 'person.Name' value doesn't change the lock... 
        person.Name = "Nancy Smith"; 
       } 
       Console.WriteLine("{0} is {1} years old.", person.Name, person.Age); 
      } 
     } 
    } 

    static void NameChange(object subject) 
    { 
     var person = subject as Person; 
     if (person == null) throw new ArgumentNullException("subject"); 
     // You should avoid locking on strings, since they are immutable. 
     if (Monitor.TryEnter(person.Name, 30) == false) 
     { 
      Console.WriteLine("Failed to obtain lock on 50 year old Nancy, because Timewarp(object) locked on string \"Nancy Drew\"."); 
     } 
     else Monitor.Exit(person.Name); 

     if (Monitor.TryEnter("Nancy Drew", 30) == false) 
     { 
      Console.WriteLine("Failed to obtain lock using 'Nancy Drew' literal, locked by 'person.Name' since both are the same object thanks to inlining!"); 
     } 
     else Monitor.Exit("Nancy Drew"); 
     if (Monitor.TryEnter(person.Name, 10000)) 
     { 
      string oldName = person.Name; 
      person.Name = "Nancy Callahan"; 
      Console.WriteLine("Name changed from '{0}' to '{1}'.", oldName, person.Name); 
     } 
     else Monitor.Exit(person.Name); 
    } 
} 

consola de salida

'this' person is locked! 
Nancy Drew is 16 years old. 
'this' person is locked! 
Nancy Drew is 17 years old. 
Failed to obtain lock on 50 year old Nancy, because Timewarp(object) locked on string "Nancy Drew". 
'this' person is locked! 
Nancy Smith is 18 years old. 
'this' person is locked! 
Nancy Smith is 19 years old. 
'this' person is locked! 
Nancy Smith is 20 years old. 
Failed to obtain lock using 'Nancy Drew' literal, locked by 'person.Name' since both are the same object thanks to inlining! 
'this' person is locked! 
Nancy Smith is 21 years old. 
'this' person is locked! 
Nancy Smith is 22 years old. 
'this' person is locked! 
Nancy Smith is 23 years old. 
'this' person is locked! 
Nancy Smith is 24 years old. 
Name changed from 'Nancy Drew' to 'Nancy Callahan'. 

Answer 2

Porque si las personas pueden obtener en su instancia de objeto (es decir, su this) puntero, entonces también pueden intentar bloquear ese mismo objeto. Ahora que podría no ser consciente de que estás de bloqueo en 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á de bloqueo "demasiado"

Por ejemplo, puede tener 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 serán bloqueadas esperando el bloqueo. Si esas funciones no necesitan acceder a la lista de miembros, provocará que otro código espere y ralentice su aplicación sin ningún motivo.

Answer 3

... y exactamente los mismos argumentos se aplican a esta construcción, así:

lock(typeof(SomeObject)) 

Answer 4

Tome un vistazo a la MSDN Tema Thread Synchronization (C# Programming Guide)

En general, lo mejor es evitar el bloqueo en un tipo público, o en el objeto instancias fuera del control de su aplicación . Por ejemplo, lock (this) puede ser problemático si se puede acceder públicamente a la instancia , porque el código más allá de su control también puede bloquearse en el objeto . Esto podría crear situaciones de interbloqueo donde dos o más subprocesos esperan el lanzamiento del mismo objeto. Bloquear en un tipo de datos público , a diferencia de un objeto, puede causar problemas por el mismo motivo . El bloqueo en cadenas literales es especialmente arriesgado porque las cadenas literales son intercaladas por el tiempo de ejecución del lenguaje común (CLR). Esto significa que hay una instancia de cualquier literal de cadena dada para todo el programa , exactamente el mismo objeto representa el literal en todos los dominios de aplicación en ejecución, en todos los subprocesos. Como resultado, un bloqueo colocado en una cadena con los mismos contenidos en cualquier parte del proceso de aplicación bloquea todas las instancias de esa cadena en la aplicación . Como resultado, es mejor bloquear un miembro privado o protegido que no está internado. Algunas clases proporcionan miembros específicamente para el bloqueo . El tipo Array, por ejemplo, proporciona SyncRoot. Muchos tipos de recopilación proporcionan un miembro SyncRoot como .

Answer 5

Porque cualquier fragmento de código que pueda ver la instancia de su clase también puede bloquear esa referencia.Desea ocultar (encapsular) su objeto de bloqueo para que solo el código que necesite referencia pueda referenciarlo. La palabra clave this se refiere a la instancia de la clase actual, por lo que cualquier cantidad de elementos podría tener referencia y podría usarse para sincronizar los hilos.

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 que su código obtenga un bloqueo oportuno o podría crear otros problemas de sincronización de subprocesos. Mejor caso: 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. Peor caso: algo usa una referencia de tu clase para hacer bloqueos y causa problemas realmente malos, muy sutiles y realmente difíciles de solucionar.

Answer 6

También hay algo de buena discusión sobre esto aquí: Is this the proper use of a mutex?

Answer 7

Lo siento chicos, pero no puedo estar de acuerdo con el argumento de que esto podría causar bloqueo callejón sin salida. Estás confundiendo dos cosas: bloqueo y hambre.

• No se puede cancelar estancamiento sin interrumpir uno de los hilos por lo que después de entrar en un callejón sin salida que no puede salir
• Starving terminará automáticamente después de uno de los hilos termina su trabajo

Here se una imagen que ilustra la diferencia.

Conclusión
Todavía se puede utilizar con seguridad si el hambre lock(this) hilo no es un problema para usted. Aún debe tener en cuenta que cuando el hilo, que está muriendo de hambre usando lock(this) termina en un candado con su objeto bloqueado, finalmente terminará en hambre eterna;)

Answer 8

Sé que este es un hilo viejo, pero porque la gente todavía puede ver esto y confiar en él, parece importante señalar que lock(typeof(SomeObject)) es significativamente peor que lock(this). Una vez dicho esto; sinceras felicitaciones 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, de grano grueso que hay. Por lo menos, una instancia de System.Type es global para un AppDomain, y .NET puede ejecutar múltiples programas en un AppDomain. Esto significa que dos programas completamente diferentes podrían causar interferencia entre sí incluso hasta el punto de crear un interbloqueo 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 todos los motivos citados. Sin embargo, hay un código ampliamente usado, relativamente respetado y aparentemente estable, como log4net, que usa el patrón de bloqueo (esto) extensivamente, aunque yo personalmente preferiría ver ese cambio de patrón.

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

Por lo que vale la pena.

Answer 9

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

Answer 10

Aquí hay un código de ejemplo que es más fácil de seguir (OMI): (funcionará en LINQPad, de referencia siguientes espacios de nombres: System.Net y System.Threading.Tasks)

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

Answer 11

Imagínese que usted tener un secretario experto en su oficina que sea un recurso compartido en el departamento. De vez en cuando, corres hacia ellos porque tienes una tarea, solo para esperar que otro de tus compañeros de trabajo no los haya reclamado. Por lo general, solo tienes que esperar un breve período de tiempo.

Como el cuidado es compartir, su gerente decide que los clientes también pueden usar la secretaria directamente. Pero esto tiene un efecto secundario: un cliente incluso podría reclamarlo mientras trabaja para este cliente y también necesita que ejecute parte de las tareas. Se produce un punto muerto, porque reclamar ya no es una jerarquía. Esto podría haberse evitado por completo al no permitir que los clientes los reclamen en primer lugar.

lock(this) es malo como hemos visto. Un objeto externo puede bloquear el objeto y, como no se controla quién está usando la clase, cualquiera puede bloquearlo ... Este es el ejemplo exacto como se describió anteriormente. De nuevo, la solución es limitar la exposición del objeto. Sin embargo, si tiene una clase private, protected o , usted ya podría controlar quién está bloqueando su objeto, porque está seguro de que ha escrito su código usted mismo. Entonces el mensaje aquí es: no lo expongas como public. Además, asegurarse de que se use un bloqueo en situaciones similares evita los interbloqueos.

Todo lo contrario de esto es bloquear los recursos que se comparten en el dominio de la aplicación, en el peor de los casos. Es como sacar a su secretaria afuera y permitir que todos los reclamen. El resultado es un caos absoluto, o en términos de código fuente: fue una mala idea; tirarlo y comenzar de nuevo. ¿Entonces cómo hacemos eso?

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: cadenas. La razón es que las cadenas se agrupan en. En otras palabras: si tiene dos cadenas que tienen los mismos contenidos en un dominio de aplicación, existe la posibilidad de que tengan exactamente el mismo puntero. Como el puntero se usa como la tecla de bloqueo, lo que básicamente se obtiene es un sinónimo de "prepararse para un comportamiento indefinido".

De forma similar, no debe bloquear objetos WCF, HttpContext.Current, Thread.Current, Singletons (en general), etc. ¿La manera más fácil de evitar todo esto? private [static] object myLock = new object();

Answer 12

bloqueo en la este puntero puede ser mala si está fijando sobre 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 crea una instancia de su clase. Por lo tanto, bloquear en en una instancia de una clase es diferente de bloquear en 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 clase nueva 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í es una carrera de cierre del programa en esta .

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í es una carrera de cierre del programa en 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 

Answer 13

Hay muy buen artículo sobre él por http://bytes.com/topic/c-sharp/answers/249277-dont-lock-type-objects Rico Mariani, arquitecto rendimiento para el tiempo de ejecución de Microsoft® .NET

Extracto:

El problema fundamental aquí es que no lo hace posee el tipo de objeto, y usted no sabe quién más podría acceder a él. En general, es una muy mala idea confiar en bloquear un objeto que no has creado y no saber a quién más podría acceder . Hacerlo invita a un punto muerto. La forma más segura es solo bloquear objetos privados.

Answer 14

Consulte el siguiente enlace que explica por qué el bloqueo (esto) no es una buena idea.

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

Así que la solución es añadir un objeto privado, por ejemplo, cuadro LockObject a la clase y coloque la región de código dentro de la instrucción de bloqueo como se muestra a continuación:

lock (lockObject) 
{ 
... 
}