Я думаю, что ваш код содержит две ошибки.
Во-первых, обычно, когда вы получаете объект из какого-то неизвестного источника, как это делает ваш конструктор, вы делаете защитную копию, чтобы быть уверенным, что он не изменен вне класса.
MyClass(List<String> list) {
this.list = new ArrayList<String>( list );
Так что, если вы сделаете это, теперь вам нужно изменить этот список где-нибудь внутри класса? Если это так, метод:
public String select() {
return list.get(currentIndex.getAndIncrement() % list.size());
не атомарно. Здесь может произойти вызов потока getAndIncrement()
, а затем выполнение модуля (%
). Затем в этот момент, если он заменяется другим потоком, который удаляет элемент из списка, старый предел list.size()
больше не будет действительным.
Я думаю, что для этого нет ничего, кроме добавления synchronized
ко всему методу:
public synchronized String select() {
return list.get(currentIndex.getAndIncrement() % list.size());
И то же самое с любым другим мутатором.
(final
, как упоминается в другом плакате, все еще требуется в полях экземпляра.)