NoSuchElementException возникает при одновременном переходе через Java ArrayList

У меня есть метод, аналогичный описанному ниже:

public void addSubjectsToCategory() {
    final List<Subject> subjectsList = new ArrayList<>(getSubjectList());
    for (final Iterator<Subject> subjectIterator =
            subjectsList.iterator(); subjectIterator.hasNext();) {
         addToCategory(subjectIterator.next().getId());
    } 
}

Когда это выполняется одновременно для одного и того же пользователя (другой экземпляр), иногда он NoSuchElementException. По моему пониманию, иногда subjectIterator.next() выполняется, когда в списке нет элементов. Это происходит только при доступе. Будет ли синхронизация методов решить эту проблему?

Трассировка стека:

java.util.NoSuchElementException: null
at java.util.ArrayList$Itr.next(Unknown Source)
at org.cmos.student.subject.category.CategoryManager.addSubjectsToCategory(CategoryManager.java:221)

Эта трассировка стека терпит неудачу в addToCategory(subjectIterator.next().getId()); линия.

Ответы

Ответ 1

Основное правило итераторов заключается в том, что базовая коллекция не должна изменяться при использовании итератора.

Если у вас есть один поток, кажется, нет ничего плохого в этом коде, пока getSubjectsList() не возвращает нуль ИЛИ addToCategory() или getId() имеют некоторые странные побочные эффекты, которые модифицируют subjectsList. Обратите внимание, однако, что вы можете переписать for-loop несколько лучше (for(Subject subject: subjectsList)...).

Судя по вашему коду, я думаю, что у вас есть еще один поток, который изменяет subjectsList. Если это так, использование SynchronizedList, вероятно, не решит вашу проблему. Насколько мне известно, синхронизация применяется только к методам List, таким как add(), remove() и т.д., И не блокирует сбор во время итерации.

В этом случае добавление synchronized к методу тоже не поможет, потому что другой поток делает свои неприятные вещи в другом месте. Если эти предположения верны, самым простым и безопасным способом является создание отдельного объекта синхронизации (например, Object lock = new Object()), а затем установка synchronized (lock) {... } вокруг этого цикла цикла, а также любого другого места в вашей программе, которая изменяет коллекцию. Это предотвратит выполнение другими изменениями, пока этот поток выполняет итерацию, и наоборот.

Ответ 2

Похоже, что другой поток вызывает метод и захватывает последний элемент, а другой поток собирается получить следующий. Поэтому, когда другой поток заканчивается и возвращается в приостановленную нить, ничего не осталось. Я предлагаю использовать ArrayBlockingQueue вместо List. Это будет блокировать потоки, если вы уже итерации.

public void addSubjectsToCategory() {
    final ArrayBlockingQueue<Subject> subjectsList = new ArrayBlockingQueue(getSubjectList());
    for (final Iterator<Subject> subjectIterator =
         subjectsList.iterator(); subjectIterator.hasNext();) {
        addToCategory(subjectIterator.next().getId());
    }
}

Есть немного морщин, которые вам, возможно, придется разобраться. ArrayBlockingQueye будет блокироваться, если он пуст или заполнен, и дождитесь, когда поток либо вставляет что-то, либо что-то вынимает, прежде чем он разблокирует и разрешит другим потокам доступ.

Ответ 3

subjectIterator.hasNext();) {

--- Представьте, что здесь происходит переключение потоков, между вызовами hasNext() и next().

addToCategory(subjectIterator.next().getId());

Что может случиться, так это следующее: если вы находитесь в последнем элементе в списке:

  • thread A вызывает hasNext(), результат равен true;
  • переключатель резьбы происходит с резьбой B;
  • thread B вызывает hasNext(), результат также верен;
  • thread B вызывает next() и получает следующий элемент из списка; теперь список пуст, потому что он был последним;
  • переключатель резьбы возвращается к резьбе A;
  • поток A уже находится внутри тела цикла for, потому что это то место, где оно было прервано, оно уже hasNext раньше, что было истинно;
  • поэтому поток A вызывает next(), который теперь не работает с исключением, потому что в списке больше нет элементов.

Итак, что вы должны делать в таких ситуациях, заключается в том, чтобы сделать операции hasNext а next вести себя по-атомному, без переключения потоков. Простая синхронизация в списке решает, действительно, проблему:

public void addSubjectsToCategory() {
    final ArrayBlockingQueue<Subject> subjectsList = new ArrayBlockingQueue(getSubjectList());
    synchronized (subjectsList) {
        for (final Iterator<Subject> subjectIterator =
            subjectsList.iterator(); subjectIterator.hasNext();) {
                addToCategory(subjectIterator.next().getId());
        }
    }
}

Обратите внимание, однако, что с этим подходом могут быть последствия для производительности. Ни один другой поток не сможет читать или писать из/в тот же список, пока итерация не закончится (но это то, что вы хотите). Чтобы решить эту проблему, вам может понадобиться переместить синхронизацию внутри цикла, только вокруг hasNext и next. Или вы можете использовать более сложные механизмы синхронизации, такие как блокировки чтения и записи.

Ответ 4

Вы можете использовать Collections.synchronizedList(list), если все, что вам нужно, это простая синхронизация вызова. Но обратите внимание, что используемый вами итератор должен находиться внутри блока Synchronized.

Ответ 5

Как я понимаю, вы добавляете элементы в список, который может быть в процессе чтения. Представьте, что список пуст, и ваш другой поток читает его. Эти проблемы могут привести к вашей проблеме. Вы никогда не могли быть уверены, что в этом списке элемент будет записан в ваш список, который вы пытаетесь прочитать.

Ответ 6

Я был удивлен, не увидев ответа, связанного с использованием CopyOnWriteArrayList или Guava ImmutableList поэтому я подумал, что добавлю здесь такой ответ.

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

Во-вторых, если ваш вариант использования таков, что вы можете добавить все в свой список вперед одним однопоточным способом, и только тогда вам нужно перебирать его параллельно, а затем рассмотреть класс Guava ImmutableList. Вы выполните это, сначала используя стандартный ArrayList или LinkedList или строитель для вашего ImmutableList. После завершения ввода однопоточной записи вы создаете экземпляр ImmutableList с помощью ImmutableList.copyOf() или ImmutableList.build(). Если ваш вариант использования позволит использовать этот шаблон записи/чтения, это, вероятно, будет вашим самым эффективным вариантом.

Надеюсь, это поможет.

Ответ 7

Я хотел бы сделать предложение, которое, вероятно, решит вашу проблему, учитывая, что это проблема параллелизма.

Если при создании метода addSubjectsToCategory() синхронизируется ваша проблема, вы обнаружите, что проблема параллелизма. Важно найти, где проблема, в противном случае предоставленная вами информация бесполезна для нас, мы не можем вам помочь.

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

public synchronized void addSubjectsToCategory(List subjectsList){
  Iterator iterator = subjectsList.iterator();
  while(iterator.hasNext())
    addToCategory(iterator.next().getId());
}

или же

//This semaphore should be used by all threads. Be careful not to create a
//different semaphore each time.
public static Semaphore mutex = new Semaphore(1);

public void addSubjectsToCategory(List subjectsList){
  Iterator<Subject> iterator = subjectsList.iterator();
  mutex.acquire();
  while(iterator.hasNext())
    addToCategory(iterator.next().getId());
  mutex.release();
}

Синхронизированный чистый, аккуратный и элегантный. У вас действительно маленький метод и создание блокировок, имхо не нужно. Синхронизированный означает, что только один поток сможет вводить метод за раз. Это означает, что вы должны использовать его только в том случае, если вы хотите, чтобы 1 поток был активным каждый раз.

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