Code Review: Синхронизация критической секции в двух потоках

25. Исправить синхронизацию критической секции между потоками

Условие задачи:
Дан сервис, метод которого одновременно вызывается из двух потоков. Часть метода является критической секцией и должна выполняться только одним потоком в каждый момент времени.

Нужно провести code review, найти ошибки в запуске потоков и синхронизации, а затем предложить корректный вариант реализации.

Код:

class Main{
void main(){
    execute(()->new Service.doSome());
    execute(()->new Service.doSome());
}
}

class Service{
    void doSome(){
        Object obj = new Object();
        //code
        synchronized(obj){
            //code
        }
        //code
    }
}

Спойлеры к решению

Подсказки

💡 Для взаимного исключения оба потока должны синхронизироваться на одном и том же объекте.
💡 Проверь, где создаётся объект-монитор.
💡 Если используется монитор экземпляра сервиса, оба потока должны работать с одним экземпляром Service.
💡 Проверь синтаксис создания объекта и вызова doSome().

Решение

Проблемы:

Object obj = new Object() создаётся при каждом вызове, поэтому потоки получают разные мониторы;
критическая секция фактически не защищена от параллельного выполнения;
new Service.doSome() — некорректный вызов конструктора и метода;
если каждый поток создаст собственный Service, синхронизация на поле экземпляра или this тоже не поможет;
блокировать весь doSome() необязательно, если защищать нужно только небольшую критическую секцию.

Как лучше исправить

Создать один объект Service и один общий монитор внутри него:

class Main {

    public static void main(String[] args) {
        Service service = new Service();

        execute(service::doSome);
        execute(service::doSome);
    }

    static void execute(Runnable runnable) {
        new Thread(runnable).start();
    }
}

class Service {

    private final Object lock = new Object();

    void doSome() {
        // code

        synchronized (lock) {
            // critical section
        }

        // code
    }
}

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

Если вся критическая логика вынесена в отдельный метод, можно синхронизироваться на this:

class Service {

    void doSome() {
        // code
        critical();
        // code
    }

    private synchronized void critical() {
        // critical section
    }
}

Такой вариант также работает, пока оба потока используют один экземпляр Service.

Важно: synchronized обеспечивает синхронизацию только внутри одной JVM. Если приложение работает в нескольких экземплярах, для общей внешней критической секции потребуется другой механизм — например, блокировка на уровне БД или распределённый lock.