Как сделать методы потокобезопасными? Java

151

14 апреля 2022, 19:00

Допустим есть подобный сервис (SomeService), методы getObject() и setPojo() вызываются из RestController (Использую Spring). Вопрос, как сделать этот сервис потокобезопасным в многопоточной среде, когда к нему будет обращаться множество пользователей? Достаточно ли добавить к методам getPojo() и setPojo() ключевое слово synchronized и заменить map = new HashMap<>() на map = new ConcurrentHashMap<>() ?

public class SomeService {
    private final Map<String, TreeSet<Pojo>> map = new HashMap<>();
    public Set<Pojo> getPojo(String PojoId) {
        return map.get(userId);
    }
    public boolean setPojo(Pojo pojo) {
        TreeSet<Pojo> set = map.get(pojo.getId());
        if (set == null) {
            set = new TreeSet<>(USER_INFO_COMPARATOR);
            return add(pojo, set);
        } else if (condition1) {
            return addPojo(pojo, set);
        } else if (condition2) {
            return updateSet(pojo, set);
        }
        System.out.println("ERROR");
        return false;
    }
    private boolean updateSet(Pojo pojo, TreeSet<Pojo> set) {
        set.pollLast();
        return addPojo(pojo, set);
    }
    private boolean addPojo(Pojo pojo, TreeSet<Pojo> set) {
        set.add(pojo);
        System.out.println("Added New");
        return true;
    }
}

Answer 1

Как написали в комментариях, синхронизация отдельных методов не поможет.

Во-первых, есть проблема с неатомарностью get-modify-set. Так же есть еще одна проблема, а именно то, что часть внутреннего состояния класса возвращается из getPojo и дальше клиент может делать с Set<Pojo> что хочет и когда хочет.

Синхронизация отдельных методов не поможет решить эти проблемы. Нужно синхронизировать, а если точнее, то сделать атомарными, более высокоуровневые операции. Первое, что нужно сделать - это спрятать от клиентов внутренности класса. Нужно изменить API класса так, чтобы полностью контролировать все модифицирующие операции, т.е. чтоб клиент не мог изменить состояние класса, без ведома класса.

Один способ это реализовать optimistic concurrency, т.е. при изменении состояния атомарно проверять, что не было конкурирующих изменений с тех пор, как клиент класса прочитал состояние класса. Можно поступить, как написал vp_arth, те реализовать метод типа boolean setIfValueIsNotChanged(key, new, old), в который нужно передавать старое значение либо какой-то другой идентификатор версии, чтобы модифицирующий метод мог проверить, что значение все еще актуальное и не было изменений. Такой подход потребует чтобы значение, которое возвращается get методами было неизменяемым (immutable), т.е. чтобы используя это значение клиент не смог изменить внутреннее состояние в обход setIfValueIsNotChanged. Это может быть копия значения хранящегося внутри или хранить значение в неизменяемой форме.

Клиент использующий setIfValueIsNotChanged должен быть готов к тому, что при вызове данного метода, будет обнаружен конфликт, и клиенту в таком случае нужно будет заново повторить высокоуровневую операцию чтение-изменение-запись.

Второй способ - это дать клиенту возможность изменять состояние, но делать это всегда в критической секции. Можно это сделать например так (на более простом примере):

public class SomeService {
   private State hiddenState;
   // это единственная функция доступная извне, которая меняет состояние
   public synchronized void changeState(Consumer<UpdatableService> callback) {
       callback.apply(new ServiceModifier());
   }
   public class UpdatableService {
       public void setState(State state) {
         hiddenState = state;    
       }
       public State getState() {
         return hiddenState;
       }  
   }
}

Теперь клиент может делать операции изменения атомарно:

SomeService service = ...;
service.changeState((UpdatableService updatableService) -> {
   State state = updatableService.getState();
   // используем state чтобы посчитать новое значение
   newState = ...;
   updatableService.setState(newState);
});

У этого подхода есть минус. Если клиент будет делать какую-то тяжеловесную операцию внутри changeState, то это будет блокировать всех других клиентов. Это плата за относительную простоту этого решения.

И еще один важный момент. Я бы очень сильно подумал, действительно ли вы хотите хранить состояние в сервисе, т.е. в памяти. Любой сбой (программный или аппаратный) приведет к потере пользовательских данных. Такие сбои это обычное дело, более того это нормальное поведение, если ваш сервис запущен на каком-нибудь облаке типа AWS или Azure (там виртуальная машина может быть остановлена и перезапущена в любой момент). Вам придется решать очень много проблем (возможно вы о них не задумывались), если вы пойдете этим путем. Т.е. если говорить коротко, то такие вещи нужно хранить в каком-то долговременном хранилище типа БД или файловой системы, а в памяти разве что кешировать. При хранении, скажем, в БД тоже, конечно же, возникнет вопрос о параллельных изменениях, и для решения есть свои методы и подходы.