c# - teclas - ¿Por qué está mal el bloqueo(esto){...}?

habilitar y deshabilitar botones en c#

Question

La documentación de MSDN 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é? ¿Será porque la cerradura se mantendrá más tiempo del necesario? ¿O hay alguna razón más insidiosa?

Answer 1

... y exactamente los mismos argumentos se aplican a este constructo también:

lock(typeof(SomeObject))

Answer 2

Aquí hay un código de ejemplo que es más sencillo de seguir (IMO): ( Funcionará en LinqPad , haga referencia a los 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 3

Aquí hay una ilustración mucho más simple (tomada de la pregunta 34 aquí ) por qué el bloqueo (esto) es malo y puede resultar en puntos muertos cuando el consumidor de su clase también trata de bloquear el objeto. A continuación, solo uno de los tres subprocesos puede continuar, 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 solucionar este problema, este tipo utilizó Thread.TryMonitor (con tiempo de espera) en lugar de bloquear:

            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

Answer 4

Bloquear en este puntero puede ser malo si 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. El motivo es que este puntero contendrá una referencia diferente a una ubicación en la memoria cada vez que se cree una instancia de su clase. Por lo tanto, bloquear sobre esto en una instancia de una clase es diferente a bloquear sobre esto 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();
    }

Crea una nueva clase como la de abajo.

 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 ejecución del programa de bloqueo en 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í es una ejecución del programa de bloqueo 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 5

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

Entonces, la solución es agregar un objeto privado, por ejemplo, lockObject a la clase y colocar la región del código dentro de la declaración de bloqueo como se muestra a continuación:

lock (lockObject)
{
...
}

Answer 6

Eche un vistazo a la sincronización de subprocesos de temas de MSDN (Guía de programación de C #)

En general, es mejor evitar el bloqueo en un tipo público o en instancias de objetos fuera del control de su aplicación. Por ejemplo, el bloqueo (esto) puede ser problemático si se puede acceder públicamente a la instancia, porque el código que está fuera de su control también puede bloquearse en el objeto. Esto podría crear situaciones de interbloqueo donde dos o más subprocesos esperan la liberación del mismo objeto . El bloqueo en un tipo de datos públicos, a diferencia de un objeto, puede causar problemas por la misma razón. El bloqueo de cadenas literales es especialmente arriesgado porque las cadenas literales están internadas por el Common Language Runtime (CLR). Esto significa que hay una instancia de cualquier cadena literal dada para todo el programa, el mismo objeto representa el literal en todos los dominios de aplicaciones en ejecución, en todos los subprocesos. Como resultado, un bloqueo colocado en una cadena con el mismo contenido en cualquier parte del proceso de la 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 de Array, por ejemplo, proporciona SyncRoot. Muchos tipos de colección también proporcionan un miembro de SyncRoot.

Answer 7

Es una mala forma usar this en sentencias de bloqueo porque generalmente está fuera de su control quién más podría estar bloqueando ese objeto.

Para planificar adecuadamente las operaciones paralelas, se debe tener especial cuidado al considerar posibles situaciones de interbloqueo, y tener un número desconocido de puntos de entrada de bloqueo lo dificulta. 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 podría afectar su corrección.

Un campo privado suele ser una mejor opción, ya que el compilador le impondrá restricciones de acceso 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 si va a adquirir un bloqueo en this menos que se haya documentado. Incluso entonces, confiar en la documentación para evitar un problema es subóptimo.

Finalmente, existe la idea errónea de que el lock(this) realidad 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 un parámetro para lock simplemente sirve como una clave . Si ya hay una cerradura en esa llave, no se puede hacer la cerradura; de lo contrario, se permite el bloqueo.

Esta es la razón por la que es malo usar cadenas como claves en las declaraciones de lock , ya que son inmutables y son compartidas / accesibles a través de partes de la aplicación. Debería usar una variable privada en su lugar, una instancia de Object bien.

Ejecute el siguiente código de 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);
    }
}

Salida de consola

''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 8

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

Answer 9

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

Extracto:

El problema básico aquí es que no posee el objeto de tipo y 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 creó y no saber a quién más podría acceder. Si lo hace, invita a punto muerto. La forma más segura es solo bloquear objetos privados.

Answer 10

Imagine que tiene una secretaria especializada en su oficina que es 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. Normalmente solo tienes que esperar un breve periodo de tiempo.

Debido a que la atención 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 trabaja para este cliente y también los necesita para ejecutar parte de las tareas. Se produce un punto muerto, porque la reclamación ya no es una jerarquía. Esto podría haberse evitado por completo al no permitirles a los clientes reclamarlos en primer lugar.

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

Lo opuesto a esto es bloquear los recursos que se comparten en todo el dominio de la aplicación, el peor de los casos. Es como poner a tu secretaria afuera y permitir que todos los reclamen. 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 hacemos eso?

Los tipos se comparten en el dominio de la aplicación, como señala la mayoría de las personas aquí. Pero hay cosas aún mejores que podemos usar: cuerdas. La razón es que las cadenas están agrupadas . 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 usa 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.Current, Thread.Current, Singletons (en general), etc. ¿La forma más fácil de evitar todo esto? private [static] object myLock = new object();

Answer 11

Lo siento, pero no estoy de acuerdo con el argumento de que bloquear esto podría causar un punto muerto. Estás confundiendo dos cosas: estancamiento y hambre.

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

Here hay una foto que ilustra la diferencia.

Conclusión
Aún puede usar el lock(this) forma segura lock(this) si la inanición de subprocesos no es un problema para usted. Aún debe tener en cuenta que cuando el hilo, que es un hilo hambriento con lock(this) termina en una cerradura que tiene su objeto bloqueado, finalmente terminará en una inanición eterna;)

Answer 12

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 necesita referenciarlo pueda hacer referencia a él. La palabra clave se refiere a la instancia de la clase actual, por lo que cualquier número de cosas podría tener referencia a ella y podría usarla para realizar la sincronización de subprocesos.

Para ser claros, esto es malo porque alguna otra parte del 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 su clase para bloquear. Caso intermedio: algo usa una referencia a su clase para hacer bloqueos y causa problemas de rendimiento. El peor de los casos: algo usa una referencia de su clase para hacer bloqueos y causa problemas realmente graves, muy sutiles, muy difíciles de depurar.

Answer 13

Porque si la gente puede obtener el puntero de su instancia de objeto (es decir, su this ), también pueden intentar bloquear ese mismo objeto. Ahora es posible que no sepan 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, podría 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, otras cosas que llamen a esas funciones se bloquearán esperando el bloqueo. Si esas funciones no necesitan acceder a la lista de miembros, estará causando que otro código espere y ralentice su aplicación sin ningún motivo.

Answer 14

Sé que este es un hilo antiguo, pero como la gente aún puede buscarlo y confiar en él, parece importante señalar que el lock(typeof(SomeObject)) es significativamente peor que el lock(this) . Una vez dicho esto; felicitaciones sinceras a Alan por señalar que el 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. Como mínimo, una instancia de System.Type es global para un dominio de aplicación, y .NET puede ejecutar múltiples programas en un dominio de aplicación. Esto significa que dos programas completamente diferentes podrían causar interferencias entre sí, incluso al punto de crear un interbloqueo si ambos intentan obtener un bloqueo de sincronización en la misma instancia de tipo.

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

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

Por lo que vale.

Answer 15

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