Нужно ли синхронизировать каждый метод в классе Singleton?

Question

В приведенном ниже примере многопоточного программирования класс Replacer может быть доступен нескольким потокам одновременно, поэтому я сделал класс singleton и синхронизировал метод getInstance().Нужно ли также синхронизировать метод replaceNum(), предполагая, что метод также будет вызываться несколькими потоками?

public class Replacer {

  private static Replacer replacer = null;
  private List<Integer> nums;

  private Replacer() {
    nums = new ArrayList<>();
  }

  public static synchronized Replacer getInstance() {
    if (replacer == null) {
      replacer = new Replacer();
    }

    return replacer;
  }

  // Do I need to make below method synchronized?
  public void replaceNum(List<Integer> newNums) {
    if (nums.size() > 0) {
      nums.remove(nums.size() - 1);
    }

    nums.addAll(newNums);
  }
}

Answer 1

Если существует несколько обращений к вызову метода (запись и чтение), тогда да, вам нужно будет использовать synchronized, поскольку ArrayList по своей природе не является поточно-ориентированной реализацией.

Что-то вроде:

public synchronized void replaceNum(List<Integer> newNums) {
    if (nums.size() > 0) {
        nums.remove(nums.size() - 1);
    }
    nums.addAll(newNums);
}

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

Answer 2

Любой метод, который изменяет состояние общего экземпляра, должен быть помечен synchronized, чтобы быть безопасным.Методы synchronized "проверяют" блокировку на экземпляре класса (this), и все синхронизированные методы получают контроль над той же самой блокировкой, поэтому использование этого ключевого слова в нескольких методах сделает эту работу.Хотя вы должны попытаться synchronize на небольшом фрагменте кода, если вы ожидаете, что он будет вызываться много раз.В общем случае для лучшей производительности следует синхронизировать как можно меньше кода.

Answer 3

(Ответ был написан для более ранней версии вопроса ).

Нет, синхронизировать его не нужно, потому что вы никогда не изменяете список структурно.

Вы добавляете в список, только если он не пустой.Поэтому вы никогда не добавляете в него первый элемент.

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

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

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

Обратите внимание, что эта реализация не синхронизирована .Если несколько потоков обращаются к экземпляру ArrayList одновременно, и хотя бы один из потоков структурно изменяет список, он должен быть синхронизирован извне.(Структурная модификация - это любая операция, которая добавляет или удаляет один или несколько элементов или явно изменяет размер базового массива; просто установка значения элемента не является структурной модификацией.)

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

Этот первый бит (, если список не пуст, удалить его последний элемент ) может быть испорчен несколькими потоками.Например, два потока могут одновременно найти список размером 1;затем один поток удаляет последний элемент перед другим;затем другой поток пытается удалить удаленный элемент.Kaboom.

Или добавление может быть испорчено, если оба потока обнаружат, что список пуст, и поэтому не пытайтесь удалить что-либо;затем добавьте, и вы получите два элемента в списке.

Даже если вы используете Collections.synchronizedList, как рекомендовано в документации ArrayList, это не решит вашу проблему.Важно понимать, что приведенная выше цитата - это то, что вы должны сделать, чтобы сохранить инварианты из ArrayList;он ничего не делает для принудительного применения инвариантов вашего кода.

Вы должны быть в состоянии выполнить эту логику атомарно .Все это делается как «одно действие» одним потоком с монопольным доступом к списку.Это достигается путем синхронизации.

Answer 4

Правило не является особенным для одиноких. Это чисто:

• Должен ли метод поддерживать несколько потоков, вызывающих его, и
• Делает ли он что-нибудь, что не получится, если несколько потоков вызовут его в Just The Wrong Time

Ответ для вашего replaceNum: «Да, ему нужна синхронизация» (и на уровне методов это имеет смысл, поскольку в основном все в нем должно быть синхронизировано), поскольку он использует методы ArrayList, которые не являются потокобезопасными. Как Javadoc говорит :

Обратите внимание, что эта реализация не синхронизирована. Если несколько потоков одновременно обращаются к экземпляру ArrayList, и хотя бы один из потоков структурно изменяет список, он должен быть синхронизирован извне. (Структурная модификация - это любая операция, которая добавляет или удаляет один или несколько элементов или явно изменяет размер базового массива; просто установка значения элемента не является структурной модификацией.)

Таким образом, этот метод должен синхронизировать доступ к этому ArrayList.

Учитывая такое общее утверждение, вы должны предполагать, что ни один из методов не является поточно-ориентированным, если в нем явно не указано, что равно . (Точно так же, если класс явно не говорит, что является поточно-безопасным, вы должны предположить, что это не так.)