lock (new object ()) - Культ груза или какой-то сумасшедший «особый случай языка»?

87

Я просматриваю код, написанный консультантом, и, хотя уже появились десятки красных флажков, я не могу осмыслить следующий фрагмент:

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

Что здесь делает lock (new object ())? Не должно иметь никакого эффекта, поскольку он всегда блокирует другой объект, но этот вид блокировки сохраняется во всем коде, даже в частях, не связанных с копированием и вставкой. Это какой-то особый случай на языке C #, который скомпилирован для чего-то, о чем я не знаю, или программист просто принял какой-то культ карго, который работал некоторое время назад?

Чарльз
источник
19
Я думаю, они очень запутались. Они, вероятно, видели это там, где new object()хранилось в поле, и это поле использовалось в lock()операторах, и они не знали, что лучше не вставлять его.
Damien_The_Unbeliever
21
Этому «консультанту» нужно кое-что объяснить ... Вы не ошиблись: этот lockкод совершенно бесполезен
Марк Грейвелл
12
@Baboon: Только если вы не тот, кто должен делать рефакторинг ...
2
Плюс, если это WinForms, то я не понимаю, зачем вообще там должна быть блокировка.
Дрю Ноукс,
7
Удалите его, а затем повторно запустите свой набор тестов на 100% покрытие кода. Что это? Предыдущий консультант не сделал ни одного?
Spacedman

Ответы:

82

Я бы не удивился, если бы это увидел кто-нибудь:

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 ??
    }
}

и подумал, что может сократить количество строк.

Я бы очень волновался, если бы это было так ...

cjk
источник
4
Он мог видеть метод под названием «newObject ()», и этот метод возвращал одноэлементный экземпляр, но он сказал: «Эй, разве в C # нет ключевых слов для этого»?
Амирам Корах
9
Это действительно звучит как работа по рефакторингу без эффективного понимания того, что происходит.
Aphelion
1
@OrangeDog: К сожалению, это невозможно, поскольку соответствующий код был написан до того, как я присоединился к компании. Теперь, когда нужно внести изменения, возможно, я смогу убедить менеджмент разрешить мне исправить код. В противном случае я не несу ответственности за какие-либо нестабильности (виноват тот, кто последний что-нибудь коснется) ...
2
@Ibruder Высшим приоритетом было бы убедить руководство в том, что им нужны системы контроля версий, автоматического тестирования и проверки. Если виноват тот, кто к чему-то прикоснулся последним, то это не похоже на то, чтобы работать в очень хорошей компании.
OrangeDog
1
Меня не очень волнует явно нарушенная блокировка - все остальные уже указали на это, но +1 только для 'или программ на iOS до 4/5' <g>
Мартин Джеймс
15

Вот аналогичный вопрос и ответ:

Блокировки обеспечивают взаимное исключение - не более одного потока могут удерживать блокировку одновременно. Блокировка идентифицируется с конкретным экземпляром объекта. Вы создаете новый объект для блокировки каждый раз, и у вас нет никакого способа сообщить любому другому потоку, чтобы он заблокировал тот же самый экземпляр объекта. Следовательно, ваша блокировка бесполезна.

JleruOHeP
источник
2

Наверное, бесполезно. Но есть шанс, что это создаст барьер памяти. Не уверен, действительно ли C # блокирует исключение или сохраняет ли он семантику упорядочения блокировки.

Benmmurphy
источник
Это было несколько лет назад, но вы полностью заслуживаете +1 за утверждение этого очевидного факта, который некоторые люди полностью игнорируют. Например, на универсальной платформе Windows нет метода MemoryBarrier () и волшебства с Interlocked .pareExchange и блокировка (new Object ()) становятся единственными способами решения некоторых проблем.
Sergey.quixoticaxis.Ivanov
Я даже не знал, что C # это позволяет. Замечательно с вашей стороны указать на это.
Все