18 марта 2010 г.

Чем плохо synchronized(this)?

Несколько лет назад, бродя по исходникам JDK я задался вопросом -- почему там так часто встречается организация блокировки через
private final Object lock = new Object();
....

synchronized(lock){
...
}

хотя для этих целей легко можно использовать synchronized(this)? Я еще понимаю, если нужно несколько мониторов для разных наборов атомарных изменений (хотя, на мой взгляд, это часто первый звоночек что объекту нужна декомпозиция), но я часто видел такой код и когда монитор только один. Какое-то время это было для меня загадкой, пока у кого-то из гуру я не встретил ответа -- использование this как монитора синхронизации может нарушать инкапсуляцию вашего объекта. Монитор синхронизации -- это штука с состоянием: у него есть владелец (owner) который может меняться. Давая возможность клиентам работать с вашим объектом вы не можете запретить им работать с его монитором -- а это может менять его состояние, и нарушать те инварианты, на которые вы рассчитывали, проектируя класс.

Я как-то сразу принял такую версию, хотя сходу не мог придумать очевидного способа как-то некорректно вмешаться в работу монитора. Вроде бы я придумал какой-то пример когда внешнее воздействие приводило к дедлоку, но вспомнить его мне не удается.

Но вот вчера у меня, наконец, сложился явный и четкий пример. Итак, код:
public class Runner {

private Thread owner = null;

public synchronized void run( final Callable task ) throws Exception {
    owner = Thread.currentThread();
    try {
        task.call();
    } finally {
        owner = null;
    }
}

public synchronized void check() {
    final boolean invariant = ( owner == null ) || ( owner == Thread.currentThread() );
    if ( !invariant ) {
       //can this code be executed?
       throw new AssertionError( "Lock is broken: " + owner + " is owner, but " + Thread.currentThread() + " is here!" );
    }
}

}

на первый взгляд кажется, что код в строках 17-18 никогда не может быть выполнен. Оба метода синхронизированы, никакой посторонний поток не может влезть, пока текущий поток внутри run(Callable). Однако, сломать этот объект крайне просто:
final Runner runner = new Runner();

final Callable task = new Callable() {
    public Object call() throws Exception {
        runner.wait( 2000 );
        return null;
    }
};

final Thread thread = new Thread( "runner" ) {
    public void run() {
        try {
            runner.run( task );
        } catch ( Exception e ) {
            throw new RuntimeException( e );
        }
    }
};

thread.start();

//ensure thread started
Thread.yield();
Thread.sleep( 100 );
//check the invariant
runner.check();

object.wait() должен вызываться внутри synchronized(object), и, на время ожидания он отпускает монитор. То есть пока поток thread ждет 2 секунды на мониторе runner этот монитор свободен, несмотря на то, что выше по стеку есть synchronized(runner). И в эти 2 секунды любой другой поток может захватить этот монитор -- что мы и делаем, демонстрируя нарушение инварианта класса.

Какой отсюда вывод? Вывод такой: если вы используете callback интерфейсы -- т.е. если ваш код выполняет внутри себя какой-то другой код, пришедший "со стороны" -- вы должны делать это либо вне синхронизации, либо использовать монитор синхронизации, до которого клиентский код не сможет добраться, вроде private final Object lock = new Object()

На данный момент я не вижу других способов (кроме callback), как "открытая" синхронизация с помощью synchronized(this) может нарушить инкапсуляцию. Если класс колбэки не использует -- можно использовать синхронизированные методы спокойно.

2 комментария:

  1. ну и еще вероятно synchronized(this) не прокатит в статичном методе?

    ОтветитьУдалить
    Ответы
    1. Нет, это, как раз, не принципиально: в статическом методе вместо this можно использовать MyClass.class.

      Удалить