Дважды проверил блокировку на C ++: новинка для временного указателя, затем назначьте его экземпляру - PullRequest
Новая
       38

Дважды проверил блокировку на C ++: новинка для временного указателя, затем назначьте его экземпляру

4 голосов
/ 17 августа 2010

Что-то не так со следующей реализацией Singleton? 

Foo& Instance() {
    if (foo) {
        return *foo;
    }
    else {
        scoped_lock lock(mutex);

        if (foo) {
            return *foo;
        }
        else {
            // Don't do foo = new Foo;
            // because that line *may* be a 2-step 
            // process comprising (not necessarily in order)
            // 1) allocating memory, and 
            // 2) actually constructing foo at that mem location.
            // If 1) happens before 2) and another thread
            // checks the foo pointer just before 2) happens, that 
            // thread will see that foo is non-null, and may assume 
            // that it is already pointing to a a valid object.
            //
            // So, to fix the above problem, what about doing the following?

            Foo* p = new Foo;
            foo = p; // Assuming no compiler optimisation, can pointer 
                     // assignment be safely assumed to be atomic? 
                     // If so, on compilers that you know of, are there ways to 
                     // suppress optimisation for this line so that the compiler
                     // doesn't optimise it back to foo = new Foo;?
        }
    }
    return *foo;
}

Ответы [ 7 ]

4 голосов
/ 17 августа 2010

Нет, вы даже не можете предположить, что foo = p; является атомным. Вполне возможно, что он может загрузить 16 бит 32-битного указателя, а затем заменить его перед загрузкой остальных.

Если другой поток проскользнет в этот момент и вызовет Instance(), вы поджарены, потому что указатель foo недействителен.

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

Foo& Instance() {
    scoped_lock lock(mutex);
    if (foo != 0)
        foo = new Foo();
    return *foo;
}

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

Другими словами, предполагая, что у вас есть этот контроль (вы не можете), просто создайте экземпляр каждого синглтона в main перед тем, как запустить другие потоки. Тогда вообще не используйте мьютекс. В этот момент у вас не возникнет проблем с многопоточностью, и вы можете просто использовать каноническую версию не заботиться о потоках: 

Foo& Instance() {
    if (foo != 0)
        foo = new Foo();
    return *foo;
}

И, да, это делает ваш код более опасным для людей, которые не удосужились прочитать ваши документы по API, но (IMNSHO) они заслуживают всего, что получают: -)

3 голосов
/ 17 августа 2010

Почему бы не сделать это простым? 

Foo& Instance()
{
    scoped_lock lock(mutex);

    static Foo instance;
    return instance;
}

Редактировать: В C ++ 11, где в язык введены потоки.Следующее является потокобезопасным.Язык гарантирует, что экземпляр инициализируется только один раз и в поточно-безопасном имении. 

Foo& Instance()
{
    static Foo instance;
    return instance;
}

Так что его лениво оценивают.Его нить безопасна.Это очень просто.Win / Win / Win.

1 голос
/ 17 августа 2010

Это зависит от того, какую библиотеку потоков вы используете.Если вы используете C ++ 0x, вы можете использовать атомарные операции сравнения и замены и записи барьеров, чтобы гарантировать двойную проверку блокировки.Если вы работаете с потоками POSIX или Windows, вы, вероятно, можете найти способ сделать это.Большой вопрос почему?Оказывается, синглтоны обычно не нужны.

0 голосов
/ 18 августа 2010

Нет ничего плохого в вашем коде.После scoped_lock в этом разделе будет только один поток, поэтому первый входящий поток инициализирует foo и return, а затем входит второй поток (если таковой имеется), он немедленно вернется, потому что foo больше не является нулевым.

РЕДАКТИРОВАТЬ: вставил упрощенный код. 

Foo& Instance() {
  if (!foo) {
    scoped_lock lock(mutex);
    // only one thread can enter here
    if (!foo)
        foo = new Foo;
  }
  return *foo;
}
0 голосов
/ 18 августа 2010

Спасибо за ваш вклад. После консультации с превосходной книгой Джо Даффи "Параллельное программирование в Windows" я подумал, что мне следует использовать приведенный ниже код. В основном это код из его книги, за исключением некоторых переименований и строки InterlockedXXX. Следующая реализация использует:

  1. volatile ключевое слово как на временные, так и на "фактические" указатели для защиты от переупорядочения из компилятора .
  2. InterlockedCompareExchangePointer для защиты от переупорядочения из ЦП .

Итак, это должно быть довольно безопасно (... верно?): 

template <typename T>
class LazyInit {
public:
    typedef T* (*Factory)();
    LazyInit(Factory f = 0) 
        : factory_(f)
        , singleton_(0)
    {
        ::InitializeCriticalSection(&cs_);
    }

    T& get() {
        if (!singleton_) {
            ::EnterCriticalSection(&cs_);
            if (!singleton_) {
                T* volatile p = factory_();
                // Joe uses _WriterBarrier(); then singleton_ = p;
                // But I thought better to make singleton_ = p atomic (as I understand, 
                // on Windows, pointer assignments are atomic ONLY if they are aligned)
                // In addition, the MSDN docs say that InterlockedCompareExchangePointer
                // sets up a full memory barrier.
                ::InterlockedCompareExchangePointer((PVOID volatile*)&singleton_, p, 0);
            }
            ::LeaveCriticalSection(&cs_);
        }
        #if SUPPORT_IA64
        _ReadBarrier();
        #endif
        return *singleton_;
    }

    virtual ~LazyInit() {
        ::DeleteCriticalSection(&cs_);
    }
private:
    CRITICAL_SECTION cs_;
    Factory factory_;
    T* volatile singleton_;
};
0 голосов
/ 17 августа 2010

оператор new в c ++ всегда включает двухэтапный процесс:
1.) выделение памяти идентично простой malloc
2.) вызвать конструктор для данного типа данных 

Foo* p = new Foo;
foo = p;

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

0 голосов
/ 17 августа 2010

Почему бы вам не использовать настоящий мьютекс, гарантирующий, что только один поток попытается создать foo? 

Foo& Instance() {
    if (!foo) {
        pthread_mutex_lock(&lock);
        if (!foo) {
            Foo *p = new Foo;
            foo = p;
        }
        pthread_mutex_unlock(&lock);
    }
    return *foo;
}

Это блокировка «тестируй и тестируй и устанавливай» со свободными читателями. Замените вышеперечисленное блокировкой чтения-записи, если вы хотите, чтобы чтение было гарантированно безопасным в среде без атомарной замены.

edit : если вам действительно нужны бесплатные читатели, вы можете сначала написать foo, а затем написать переменную флага fooCreated = 1. Проверка fooCreated != 0 безопасна; если fooCreated != 0, то foo инициализируется. 

Foo& Instance() {
    if (!fooCreated) {
        pthread_mutex_lock(&lock);
        if (!fooCreated) {
            foo = new Foo;
            fooCreated = 1;
        }
        pthread_mutex_unlock(&lock);
    }
    return *foo;
}
...