lock (nuevo objeto ()) - ¿Culto a la carga o algún loco “caso especial de lenguaje”?

87

Estoy revisando un código escrito por un consultor, y aunque ya han aparecido docenas de banderas rojas, no puedo entender el siguiente fragmento:

private void foo()
{
    if (InvokeRequired)
    {
        lock (new object())
        {
            if (m_bar!= null)
                Invoke(new fooDelegate(foo), new object[] { });
        }
    }
    else
    {
        if(OnBazChanged != null)
            OnBazChanged();
    }
}

¿Qué está haciendo lock (new object ()) aquí? No debería tener ningún efecto, ya que siempre se bloquea en otro objeto, pero este tipo de bloqueo es persistente en todo el código, incluso en partes que no se copian y pegan. ¿Es este un caso especial en el lenguaje C # que está compilado para algo que no conozco, o el programador simplemente adoptó algún culto de carga que funcionó hace algún tiempo?

Charles
fuente
19
Creo que están muy confundidos. Probablemente lo vieron donde new object()estaba almacenado en un campo, y ese campo se usó en las lock()declaraciones, y no sabían mejor no alinearlo.
Damien_The_Unbeliever
21
Ese "consultor" tiene algunas explicaciones que hacer ... no se equivoca: ese lockcódigo es completamente inútil
Marc Gravell
12
@Baboon: Solo si no eres tú quien tiene que hacer la refactorización ...
2
Además, si se trata de WinForms, entonces no veo por qué debería haber un candado allí.
Drew Noakes
7
Elimínelo, luego vuelva a ejecutar su conjunto de pruebas de cobertura de código al 100%. ¿Que es eso? ¿El consultor anterior no hizo uno?
Hombre espacial

Respuestas:

82

No me sorprendería que fuera alguien que viera esto:

private readonly object lockObj = new object();

private void MyMethod()
{
    lock(lockObj)
    {
        // do amazing stuff, so amazing it can only run once at a time
        // e.g. comands on the Mars Rover, or programs on iOS pre 4 / 5 ??
    }
}

y pensó que podría reducir el número de líneas.

Sin embargo, estaría muy preocupado si ese fuera el caso ...

cjk
fuente
4
Podría haber visto un método llamado "newObject ()" y este método devolvió una instancia de singleton, pero dijo "oye, ¿no tiene c # palabras clave para eso"?
Amiram Korach
9
De hecho, suena como un trabajo de refactorización sin comprender efectivamente lo que estaba sucediendo.
Aphelion
1
@OrangeDog: Lamentablemente, eso es imposible, ya que el código en cuestión se escribió antes de que me uniera a la empresa. Ahora que hay cambios que hacer, tal vez pueda convencer a la gerencia de que me permita corregir el código. De lo contrario, no me haré responsable de ninguna inestabilidad (el último en tocar algo es el culpable) ...
2
@Ibruder La prioridad más alta sería convencer a la gerencia de que necesitan control de versiones, pruebas automatizadas y sistemas de revisión. Si el último en tocar algo es el culpable, entonces no parece una muy buena compañía para la que trabajar.
OrangeDog
1
No me importa mucho el bloqueo obviamente roto: todos los demás ya lo han señalado, pero +1 solo para 'o programas en iOS pre 4/5' <g>
Martin James
15

Aquí hay una pregunta y una respuesta similares:

Los candados aseguran la exclusión mutua: no más de un hilo puede sujetar el candado al mismo tiempo. El bloqueo se identifica con una instancia de objeto específica. Está creando un nuevo objeto para bloquear cada vez y no tiene ninguna forma de informar a ningún otro hilo para bloquear exactamente esa misma instancia de objeto. Por lo tanto, su bloqueo es inútil.

JleruOHeP
fuente
2

Probablemente sea inútil. Pero existe la posibilidad de que esté ahí para crear una barrera de memoria. No estoy seguro de si c # bloquea la elisión o si conserva la semántica de ordenación del bloqueo.

benmmurphy
fuente
Fue hace unos años, pero hombre, te mereces +1 por afirmar este hecho obvio que algunas personas ignoraron por completo. Por ejemplo, en la Plataforma universal de Windows no existe el método MemoryBarrier () y la magia con Interlocked.CompareExchange y lock (new Object ()) se convierten en las únicas formas de solucionar algunos problemas.
Sergey.quixoticaxis.Ivanov
Ni siquiera sabía que C # permitía esto. Genial de tu parte para señalarlo.
Todos