Потокобезопасный конструктор копирования / оператор присваивания - PullRequest
7 голосов
/ 28 октября 2010

Скажем, мы хотим сделать класс A поточно-ориентированным, используя std::mutex.У меня есть мой конструктор копирования и оператор присваивания, аналогичный приведенному ниже коду:

#include <mutex>

class A {
private:
  int i;
  mutable std::mutex mtx;

public:
  A() : i(), mtx() { }

  A(const A& other) : i(), mtx()
  {
    std::lock_guard<std::mutex> _lock(other.mtx);
    i = other.i;
  }

  A& operator=(const A& other)
  {
    if (this!=&other) {
      std::lock_guard<std::mutex> _mylock(mtx), _otherlock(other.mtx);
      i = other.i;
    }
    return *this;
  }

  int get() const
  {
    std::lock_guard<std::mutex> _mylock(mtx);
    return i;
  }
};

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

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

Спасибо

ПРИМЕЧАНИЯ :

Это всего лишь пример.У меня может быть произвольное количество переменных-членов любого типа, оно не обязательно должно быть просто int.

После комментария Мартина Йорка о возможной взаимоблокировке, это обновленная версия, в которой используются функции копирования иswap (копирование elision также возможно, но оно не будет эффективно обрабатывать случай самостоятельного назначения).

Я также изменил int на T, поэтому люди не могут предположить, что это POD.

template<typename T>
class A {
private:
  T t;
  mutable std::mutex mtx;

public:
  A() : t(), mtx() { }

  A(const A& other) : t(), mtx()
  {
    std::lock_guard<std::mutex> _lock(other.mtx);
    t = other.t;
  }

  A& operator=(const A& other)
  {
    if (this!=&other) {
      A tmp(other);
      std::lock_guard<std::mutex> _lock(mtx);
      std::swap(t, tmp.t);
    }
    return *this;
  }

  T get() const
  {
    std::lock_guard<std::mutex> _lock(mtx);
    return t;
  }

};

Ответы [ 5 ]

4 голосов
/ 13 декабря 2013

Старый вопрос, новый ответ:

Imho, лучший способ справиться с проблемой тупиковой блокировки оператора присваивания оригинальной копии:

  A& operator=(const A& other)
  {
    if (this!=&other) {
      std::unique_lock<std::mutex> _mylock(mtx, std::defer_lock),
                                   _otherlock(other.mtx, std::defer_lock);
      std::lock(_mylock, _otherlock);
      i = other.i;
    }
    return *this;
  }

т.е. используйте std::lock(L1, L2) для одновременной блокировки двух взаимных блокировок, не опасаясь тупиковой ситуации. Вероятно, это будет более высокая производительность, чем идиома копирования / обмена, особенно если данные члена состоят из std::vector, std::string или типов, которые содержат векторы и / или строки.

В C ++ 1y (мы надеемся, что y равно 4), есть новый заголовок <shared_mutex>, обеспечивающий возможность блокировки чтения / записи, который может обеспечить повышение производительности (тестирование производительности будет необходимо для конкретных случаи использования, чтобы подтвердить это). Вот как это будет использоваться:

#include <mutex>
#include <shared_mutex>

class A {
private:
  int i;
  mutable std::shared_mutex mtx;

public:
  A() : i(), mtx() { }

  A(const A& other) : i(), mtx()
  {
    std::shared_lock<std::shared_mutex> _lock(other.mtx);
    i = other.i;
  }

  A& operator=(const A& other)
  {
    if (this!=&other) {
      std::unique_lock<std::shared_mutex> _mylock(mtx, std::defer_lock);
      std::shared_lock<std::shared_mutex> _otherlock(other.mtx, std::defer_lock);
      std::lock(_mylock, _otherlock);
      i = other.i;
    }
    return *this;
  }

  int get() const
  {
    std::shared_lock<std::shared_mutex> _mylock(mtx);
    return i;
  }
};

т.е. это очень похоже на исходный код (модифицированный для использования std::lock, как я делал выше). Но тип мьютекса члена теперь std::shared_mutex вместо std::mutex. И при защите const A (и при условии отсутствия изменяемых членов, кроме мьютекса), нужно только заблокировать мьютекс в «режиме совместного использования». Это легко сделать с помощью shared_lock<shared_mutex>. Когда вам нужно заблокировать мьютекс в «эксклюзивном режиме», вы можете использовать unique_lock<shared_mutex> или lock_guard<shared_mutex>, в зависимости от ситуации, и таким же образом, как если бы вы использовали это средство с std::mutex.

В частности, обратите внимание, что теперь многие потоки могут одновременно копировать конструкции из одного A или даже копировать присваивать из того же A. Но только один поток все еще может скопировать присвоить в одно и то же A за один раз.

2 голосов
/ 28 октября 2010

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

  • Если доступ к объектам осуществляется из нескольких потоков, то вам (дополнительно) необходимо управлять временем жизни объектов, которым нельзя управлять изнутри объектов.
  • Для управления временем жизни вам уже нужен хотя бы один объект-внешний замок, так что лучше используйте его.
  • Эта схема имеет смысл только для объектов single-get () - если ваш объект имеет более (чем один член и более), чем одна get() функция, то чтение из объекта может / будет привести к противоречивым данным.

Получение правильного многопоточного кода не сводится к тому, чтобы убедиться, что ничто не «падает» и отдельные объекты остаются в согласованном состоянии. И если вам (кажется, вам) нужна вышеуказанная схема, вы можете подумать, что вы экономите, когда ваше приложение все еще делает что-то не так.

Что касается деталей реализации: поскольку для этого вы уже используете C ++ 0x, вам также следует реализовать соответствующие операции перемещения.

1 голос
/ 28 октября 2010

Я не авторитет в этом, потому что многопоточность сложна, но пока выглядит нормально .Кстати, вы, вероятно, имели в виду

std::lock_guard<std::mutex>

и в copy-ctor:

A(const A& other) : mtx()
{
  std::lock_guard<std::mutex> _lock(other.mtx);
  i = other.i;
}

Еще один способ обеспечения безопасности потоков для other - это использование только «безопасных» геттеров для доступаэто, хотя это не будет вести себя как ожидалось, когда вызывается несколько геттеров.Но остерегайтесь ссылок!

0 голосов
/ 28 октября 2010

это более правильно, но не совсем надежно:

#include <mutex>

class A {
private:
    int i;
    std::mutex mtx;

public:
    A() : i(0), mtx() {
    }
    /* this is one option for implementation, but would be rewritten when there are more ivars in order to reduce acquisition counts */
    A(A& other) : i(other.get()), mtx() {
    }

    ~A() {
        /* unsafe if subclassed, also useful for determining whether objects are destroyed prematurely (e.g., by their containers */
        std::lock_guard<std::mutex> _mylock(this->mtx);
    }

    A& operator=(A& other) {
        std::lock_guard<std::mutex> _mylock(this->mtx);
        std::lock_guard<std::mutex> _otherlock(other.mtx);
        this->i = other.i; /* you could call other.get() and bypass the lock_guard, but i'm assuming there's really more work to be done here */
        return *this;
    }

    int get() {
        std::lock_guard<std::mutex> _mylock(this->mtx);
        return this->i;
    }
private:
    /* prohibited */
    A(const A& other);
    /* also prohibited */
    A& operator=(const A& other);
};
0 голосов
/ 28 октября 2010

Вы на самом деле этого не видите, потому что стандартные средства потоков чрезвычайно новы, и я не знаю ни одного компилятора, который их поддерживает, - вы бы продолжили искать примеры boost :: thread.Кроме того, ваше необоснованное использование синхронизации, вероятно, приведет к снижению производительности, но это только мое мнение.

...