Многопоточная корректность: использование синхронизированного блока

Question

Я использую библиотеку CMU Sphinx Speech Recognizer ( Ссылка на источник ), в которой используются блоки synchronized.

Один пример блока из RecognizerTask:

Event mailbox;

[...]

public void start() {
    synchronized (this.mailbox) {
        this.mailbox.notifyAll();
        this.mailbox = Event.START;
    }
}

Код работает без проблем, однако BugFinder выдает следующее предупреждение:

Ошибка: синхронизация с RecognizerTask.mailbox при тщетной попытке защитить его

Этот метод синхронизируется пополе в том, что кажется попыткой защиты от одновременных обновлений этого поля.Но защита поля получает блокировку на объекте, на который делается ссылка, а не на поле.Это может не обеспечить взаимное исключение, в котором вы нуждаетесь, и другие потоки могут получать блокировки для объектов, на которые имеются ссылки (для других целей).Примером такого шаблона может быть:

private Long myNtfSeqNbrCounter = new Long(0);
private Long getNotificationSequenceNumber() {
     Long result = null;
     synchronized(myNtfSeqNbrCounter) {
         result = new Long(myNtfSeqNbrCounter.longValue() + 1);
         myNtfSeqNbrCounter = new Long(result.longValue());
     }
     return result;
 }

Если честно, я не совсем понимаю описание ошибки и что в этом случае должно быть неверным.Является ли глобальная переменная не полем?А если нет, как я могу улучшить код?

/ edit: Это единственная часть, где Event.wait() называется:

Event todo = Event.NONE;
        synchronized (this.mailbox) {
            todo = this.mailbox;
            /* If we're idle then wait for something to happen. */
            if (state == State.IDLE && todo == Event.NONE) {
                try {
                    //Log.d(getClass().getName(), "waiting");
                    this.mailbox.wait();
                    todo = this.mailbox;
                    //Log.d(getClass().getName(), "got" + todo);
                } catch (InterruptedException e) {
                    /* Quit main loop. */
                    //Log.e(getClass().getName(), "Interrupted waiting for mailbox, shutting down");
                    todo = Event.SHUTDOWN;
                }
            }
            /* Reset the mailbox before releasing, to avoid race condition. */
            this.mailbox = Event.NONE;
        }

Этот кодфактически используя оператор synchronized.Имеет ли смысл вообще его использовать?

Answer 1

Я не думаю, что это применимо в вашем случае. У вас есть вызов notifyAll(), что означает, что где-то в коде других потоков есть соответствующий wait() вызов:

synchronized (this.mailbox) {
    this.mailbox.wait();        
}

означает, что другой поток снимет блокировку в ожидании уведомления.

Ваш инспектор кода, вероятно, смущен строкой:

this.mailbox = Event.START;

означает, что вы можете одновременно изменять этот объект, например, если другой поток попытается установить блокировку на this.mailbox, он увидит другой объект. Тем не менее, я думаю, что так как:

1. this.mailbox виден во всем мире
2. присвоения ссылок являются атомарными
3. замок порождает забор

все потоки должны постоянно обновлять представление объекта синхронизации.

Answer 2

синхронизированный блок «захватывает» блокировку для данного объекта, в вашем случае для объекта, обозначенного mailbox.Как только вы измените переменную mailbox, чтобы она указывала на другой объект, другие потоки могут без проблем «захватить» блокировку для этого объекта, поскольку она не взята.

Обратите внимание, что блокировка предназначена для объектов,и не для справок!

Теперь рассмотрим следующий [псевдокод]:

synchronised (myObject) { 
  myObject = new Object();
  i += 5; //assume i is an instance variable
}

здесь практически нет блокировки!каждый поток создает новый объект в блоке блокировки, и модификация i не синхронизируется!