Код, который вы реализовали, не создает точку синхронизации для того, кто получает доступ к списку через getBookIds(), что означает, что он может видеть противоречивые данные.Кроме того, кто-то, кто получил список через getBookIds(), должен выполнить внешнюю синхронизацию перед доступом к списку.Ваш вопрос также не показывает, как вы на самом деле используете класс SynchronizedBookIdsAggregator, что оставляет нам недостаточно информации, чтобы полностью ответить на ваш вопрос.
Ниже была бы более безопасная версия класса:
public class SynchronizedBookIdsAggregator {
private List<String> bookIds;
public SynchronizedBookIdsAggregator() {
bookIds = new ArrayList<String>();
}
public void updateBookIds(List<String> ids){
synchronized (this) {
bookIds.addAll(ids);
}
}
public List<String> getBookIds() {
// synchronized here for memory visibility of the bookIds field
synchronized(this) {
return bookIds;
}
}
public void setBookIds(List<String> bookIds) {
// synchronized here for memory visibility of the bookIds field
synchronized(this) {
this.bookIds = bookIds;
}
}
}
Как упоминалось ранее, в приведенном выше коде все еще есть потенциальная проблема с некоторым потоком, обращающимся к ArrayList после того, как он был получен getBookIds().Поскольку сам ArrayList не синхронизирован, доступ к нему после извлечения должен быть синхронизирован на выбранном объекте защиты:
public class SomeOtherClass {
public void run() {
SynchronizedBookIdsAggregator aggregator = getAggregator();
List<String> bookIds = aggregator.getBookIds();
// Access to the bookIds list must happen while synchronized on the
// chosen guard object -- in this case, aggregator
synchronized(aggregator) {
<work with the bookIds list>
}
}
}
Я могу представить себе использование Collections.newSynchronizedList как часть конструкции этого агрегатора,но это не панацея.Проект параллелизма действительно требует понимания основных проблем, а не «выбора правильного инструмента / коллекции для работы» (хотя последний не является неважным).
Другой потенциальный вариант, на который стоит обратить внимание, - CopyOnWriteArrayList.
Как упоминалось в skaffman, может быть лучше вообще вообще не разрешать прямой доступ к списку bookIds (например, удалить метод получения и установки).Если вы обязуете, чтобы весь доступ к списку выполнялся с помощью методов, написанных в SynchronizedBookIdsAggregator, то SynchronizedBookIdsAggregator может обеспечить все управление параллелизмом списка.Как показывает мой ответ выше, предоставление потребителям агрегатора возможности использовать «геттер» для получения списка создает проблему для пользователя этого списка: для написания правильного кода им необходимо знать стратегию синхронизации / защитный объект, и, более того, онитакже необходимо использовать эти знания для активной внешней и правильной синхронизации.
Относительно вашего второго подхода.То, что вы показали, выглядит технически правильно (хорошо!).
Но , возможно, в какой-то момент вы тоже будете читать с masterBookIds?И вы не показываете и не описываете эту часть программы!Поэтому, когда вы начинаете думать о том, когда и как вы собираетесь читать masterBookIds (т. Е. Возвращаемое значение fetchMasterListOfBookIds()), просто не забывайте также учитывать проблемы параллелизма!:)
Если вы убедитесь, что все задачи / рабочие потоки завершены до того, как вы начнете читать masterBookIds, вам не нужно делать ничего особенного.
Но, по крайней мере, в показанном вами коде вы не гарантируете этого.
Обратите внимание, что libraryBookIdsExecutor.shutdown() возвращается немедленно.Поэтому, если вы начнете использовать список masterBookIds сразу после возврата fetchMasterListOfBookIds(), вы будете читать masterBookIds , в то время как ваши рабочие потоки активно записывают в него данные, и это влечет за собой некоторые дополнительные соображения.
Может быть, это то, что вы хотите - может быть, вы хотите прочитать коллекцию во время ее записи, чтобы показать результаты в реальном времени или что-то в этом роде.Но тогда вы должны рассмотреть возможность правильной синхронизации коллекции, если вы хотите выполнить итерацию по ней во время записи.
Если вы просто хотите убедиться, что все записи в masterBookIds рабочими потоками завершены доfetchMasterListOfBookIds() возвращает, вы можете использовать ExecutorService.awaitTermination (в сочетании с .shutdown(), который вы уже звоните).