Этот класс использует AtomicBooleans.Это потокобезопасно?

Question

Я не люблю блокировать свой код с помощью синхронизировано(это), поэтому я экспериментирую с использованием Атомные логические значения.В фрагменте кода XMPPConnectionIF.connect() устанавливает сокетное соединение с удаленным сервером.Обратите внимание, что переменная _подключение используется только в соединять() метод;тогда как _связанный используется во всех других методах, которые должны использовать _xmppConn.Мои вопросы перечислены после фрагмента кода ниже.

private final AtomicBoolean _connecting = new AtomicBoolean( false );
private final AtomicBoolean _connected = new AtomicBoolean( false ); 
private final AtomicBoolean _shuttingDown = new AtomicBoolean( false ); 
private XMPPConnection _xmppConn;
/**
 * @throws XMPPFault if failed to connect
 */
public void connect() 
{
    // 1) you can only connect once
    if( _connected.get() )
        return;

    // 2) if we're in the middle of completing a connection, 
    //    you're out of luck
    if( _connecting.compareAndSet( false, true ) )
    {
        XMPPConnectionIF aXmppConnection = _xmppConnProvider.get();
        boolean encounteredFault = false;

        try
        {
            aXmppConnection.connect(); // may throw XMPPException
            aXmppConnection.login( "user", "password" ); // may throw XMPPException
            _connected.compareAndSet( false, true );
            _xmppConn = aXmppConnection;
        }
        catch( XMPPException xmppe )
        {
            encounteredFault = true;
            throw new XMPPFault( "failed due to", xmppe );
        }
        finally
        {
            if( encounteredFault )
            {
                _connected.set( false );
                _connecting.set( false );
            }
            else
                _connecting.compareAndSet( true, false );
        }
    }
}

1. Судя по моему коду, является ли потокобезопасным до такой степени, что если два потока попытаются вызвать соединять() при этом допускается только одна попытка подключения.

2. В последнем блоке я последовательно выполняю два AtomicBoolean.set(..), возникнет ли проблема, поскольку во время промежутка между этими двумя атомарными вызовами некоторые потоки могут вызвать _connected.get() другими методами?

3. Когда используешь _xmppConn, я должен сделать синхронизировано( _xmppConn) ?

ОБНОВЛЯТЬ В метод добавлен отсутствующий вызов входа в систему.

Answer 1

1. Да.Переменная _connecting действует как блокировка типа «проверка и установка», которая предотвращает несколько одновременных попыток подключения.

2. Нет проблем — даже если другой поток прочитает _connected между операциями записи, _connecting предотвратит его попытку одновременного подключения.

3. Да, если предположить, что его методы еще не являются потокобезопасными.

При этом ваш метод Connect() в его нынешнем виде свел бы меня с ума, поскольку он не обязательно подключается или выдает исключение.Вы могли бы добавить циклический цикл, но это не очень хорошо подходит, потому что для всех сетевых переходов от многопроцессорной машины, кроме самых коротких, это будет более эффективно.Более того, примитивы низкоуровневого параллелизма гораздо более подвержены ошибкам, чем синхронизированные. сильно рекомендую вам придерживаться синхронизации.

Answer 2

Имейте в виду, что использование 3 AtomicBooleanэто нет то же самое, что защитить эти три переменные одной блокировкой.Мне кажется, что состояние этих переменных представляет собой единое состояние объекта и, следовательно, они должны охраняться одной и той же блокировкой.В вашем коде, использующем атомарные переменные, разные потоки могут обновлять состояние _connected, _connecting, и _shuttingDown независимо — использование атомарных переменных только гарантирует, что доступ к такой же переменная синхронизируется между несколькими потоками.

Тем не менее, я не думаю, что синхронизация на this это то, что вы хотите сделать.Вам нужно только синхронизировать доступ к состоянию соединения.Что вы можете сделать, так это создать объект, который будет использовать в качестве блокировки для этого состояния, не включая монитор. this.А именно:

class Thing {
  Boolean connected;
  Boolean connecting;
  Boolean shuttingDown;
  Object connectionStateLock = new Object();

  void connect() {
    synchronized (connectionStateLock) {
      // do something with the connection state.
    }
  }

  void someOtherMethodThatLeavesConnectionStateAlone() {
    // free range thing-doing, without getting a lock on anything.
  }
}

Если вы занимаетесь параллельным программированием на Java, я настоятельно рекомендую прочитать Java-параллелизм на практике.

Answer 3

Я думаю, что другие адекватно освещают правильность в своих комментариях.Мой единственный дополнительный комментарий: меня немного беспокоит место релиза в финале.Похоже, вам действительно захочется обернуть туда весь блок (включая вызов _xmppConnProvider.get()) в try { }finally { }, который гарантировал бы, что вы всегда снимете блокировку.В противном случае там может произойти какое-то непроверенное исключение, которое оставит вас в невосстановимом состоянии.

Стилистически я считаю, что этот код гораздо сложнее понять, чем простое использование Synchronized/Lock для достижения взаимного исключения.Я бы начал с кода, о котором легко рассуждать, и усложнил бы его только в том случае, если вы сможете доказать, что это горячая точка.

Answer 4

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

Рассмотрим, например, настройку _связанный to true выполняется до того, как метод Connect() будет полностью выполнен?Другой поток может подумать, что вы подключены, хотя это не так.Это всего лишь предположение — я не уверен, что конкретная проблема вообще может возникнуть.

Я хочу сказать, что тот тип блокировки, который вы пытаетесь сделать, чрезвычайно сложно выполнить правильно.Придерживайтесь синхронизации или используйте замки в java.util.concurrent.locks упаковка.

Answer 5

1. Да, он определенно доволен.поскольку _connecting.compareAndSet( false, true ) позволит войти только одному потоку.

2. Вам не нужно устанавливать _connected.set(false);поскольку ему никогда не присваивается значение true, если произошло исключение.Да, это возможно не из-за преемственности, но до тех пор, пока вы не установите подключение к ложному, другие потоки, пытающиеся подключиться, не будут думать, что соединение выполняется.

3. Да, если xmppConn не является потокобезопасным.