Безопасно ли это решение для ConcurrentModificationException?

#java #multithreading #concurrentmodification

#java #многопоточность #concurrentmodification

Вопрос:

У меня есть 2-й поток, который я использую для отправки сообщений с использованием OSC. Из основного потока, в который я добавляю сообщения, у меня возникла проблема с ConcurrentModificationException.

Что я сделал, чтобы это исправить, так это создал новый список с добавляемыми сообщениями. Во 2-м потоке я добавляю эти сообщения в список, который хочу отправить.

На данный момент он работает без каких-либо проблем, но мне интересно, это удача? Другими словами, может ли быть так, что изменение для запуска в ConcurrentModificationException все еще существует, но сейчас оно действительно мало, или я действительно решил проблему?

  public void run() {


    while (running) {

        toSend.addAll(newMessages);
        newMessages.clear();

        Iterator<OSCPriorityMessage> itr = toSend.iterator();

        while (itr.hasNext()) {
            OSCPriorityMessage msg = itr.next();
            oscP5.send(msg, netAddress);
            itr.remove();
        }


        try {
            sleep((long)(waitTime));
        }
        catch (Exception e) {

        }

    }

}
 

Здесь я добавляю сообщения для отправки:

 public void send(OSCPriorityMessage msg) {


    newMessages.add(msg);
}
 

Комментарии:

1. Вы больше не получите исключение, потому что единственный поток, обновляющий список, по которому вы выполняете итерацию, — это тот же поток, который выполняет итерацию списка. Пока вы синхронизируете доступ к списку «новые сообщения», этот шаблон подходит.

2. Тем не менее, вы могли бы рассмотреть возможность использования a BlockingQueue , который является предпочтительным для реализации шаблона производитель / потребитель (что вы и пытаетесь сделать).

Ответ №1:

Вам все равно нужно синхронизировать newMessages , где бы вы к нему ни обращались. Здесь мы видим два места: 1) добавление к нему и 2) копирование его в toSend , а затем очистка. Может быть, их больше.

 public void send(OSCPriorityMessage msg) {

    synchronized(newMessages){
       newMessages.add(msg);
    }
}
 

Чтобы было ясно, что toSend это временный локальный список, предназначенный только для использования в процессе отправки, объявите его как локальную переменную.

Вместо того , чтобы

 toSend.addAll(newMessages);
newMessages.clear();
 

вы можете сделать

 ArrayList<OSCPriorityMessage> toSend;
synchronized(newMessages){
   toSend = new ArrayList<>(newMessages);
   newMessages.clear();
}
 

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


Теперь, когда у вас есть свежее toSend в каждой итерации цикла, вам больше не нужно удалять элементы, и вы можете отказаться от всего итератора, заменив его простым циклом:

 for (OSCPriorityMessage msg: toSend){
    oscP5.send(msg, netAddress);
}
 

И, наконец, как предлагает @dlev, взгляните на существующие потокобезопасные реализации очереди в java.util.concurrent пакете.

Комментарии:

1. Я мог бы также использовать toSend.addAll(newMessages) в синхронизированном блоке, верно?

2. да. Просто сохраните его в синхронизированном блоке. Мне нравится использовать конструктор для этого, но addAll , вероятно, его легче читать.