Двойное освобождение или повреждение (out) в операторе присваивания

Я работаю над параллельным кодом. В моей основной функции у меня есть цикл по времени, и в начале мне нужно скопировать класс с помощью оператора присваивания. Но каким-то образом на 4-м шаге на одном из процессоров возникает ошибка двойного освобождения или повреждения, а на других все в порядке; и ошибка на std::set и set::map. Ниже часть кода и основной цикл.

    class Mesh
    {
      public:

        const Mesh &operator=(const Mesh &mesh);

        std::set<size_t> ghostSet;
        std::map<size_t, size_t> localIndex;
    }

Оператор присваивания:

    const Mesh &operator=(const Mesh &mesh)
    {
      std::set<size_t>().swap(ghostSet);  ///BUG here
      std::map<size_t, size_t>().swap(localIndex); /// BUG sometimes here
      for(auto const &it : mesh.localIndex)
        localIndex[it.first] = it.second;
      for(auto const &it : mesh.ghostSet)
        ghostSet.insert(it);
      return *this;
    }

основная функция:

    int main(int argc, char *argv[])
    {
      Mesh ms, ms_gh;
      /// Some operation to ms;
      for(size_t t = 0; t != 10; t++)
      {
        /// Some operation to ms;
        ms_gh = ms;
        /// Some operation to ms_gh;
      }
    }

    #0  0x00002aaab2405207 in raise () from /lib64/libc.so.6
    #1  0x00002aaab24068f8 in abort () from /lib64/libc.so.6
    #2  0x00002aaab2447cc7 in __libc_message () from /lib64/libc.so.6
    #3  0x00002aaab2450429 in _int_free () from /lib64/libc.so.6
    #4  0x000000000041bfba in __gnu_cxx::new_allocator<std::_Rb_tree_node<unsigned long> >::deallocate (this=07fffffff8b50, __p=0x7131c0)
at /usr/include/c++/4.8.2/ext/new_allocator.h:110
    #5  0x000000000041835c in std::_Rb_tree<unsigned long, unsigned long, std::_Identity<unsigned long>, std::ess<unsigned long>, std::allocator<unsigned long> >::_M_put_node (this=0x7fffffff8b50, __p=0x7131c0)
at /usr/include/c++/4.8.2/bits/stl_tree.h:374
    #6  0x000000000041276e in std::_Rb_tree<unsigned long, unsigned long, std::_Identity<unsigned long>, std::ess<unsigned long>, std::allocator<unsigned long> >::_M_destroy_node (this=0x7fffffff8b50, __p=0x7131c0)
at /usr/include/c++/4.8.2/bits/stl_tree.h:422
    #7  0x000000000040c8ad in std::_Rb_tree<unsigned long, unsigned long, std::_Identity<unsigned long>, std::ess<unsigned long>, std::allocator<unsigned long> >::_M_erase (this=0x7fffffff8b50, __x=0x7131c0)
at /usr/include/c++/4.8.2/bits/stl_tree.h:1127
    #8  0x000000000040c88a in std::_Rb_tree<unsigned long, unsigned long, std::_Identity<unsigned long>, std::ess<unsigned long>, std::allocator<unsigned long> >::_M_erase (this=0x7fffffff8b50, __x=0x72f410)
at /usr/include/c++/4.8.2/bits/stl_tree.h:1125
    #9  0x000000000040c88a in std::_Rb_tree<unsigned long, unsigned long, std::_Identity<unsigned long>, std::ess<unsigned long>, std::allocator<unsigned long> >::_M_erase (this=0x7fffffff8b50, __x=0x72b760)
at /usr/include/c++/4.8.2/bits/stl_tree.h:1125
    #10 0x000000000040c88a in std::_Rb_tree<unsigned long, unsigned long, std::_Identity<unsigned long>, std::ess<unsigned long>, std::allocator<unsigned long> >::_M_erase (this=0x7fffffff8b50, __x=0x70fce0)
at /usr/include/c++/4.8.2/bits/stl_tree.h:1125
    #11 0x00000000004080c4 in std::_Rb_tree<unsigned long, unsigned long, std::_Identity<unsigned long>, std::ess<unsigned long>, std::allocator<unsigned long> >::~_Rb_tree (this=0x7fffffff8b50, __in_chrg=<optimized ut>)
at /usr/include/c++/4.8.2/bits/stl_tree.h:671
    #12 0x0000000000407bbc in std::set<unsigned long, std::less<unsigned long>, std::allocator<unsigned long> ::~set (this=0x7fffffff8b50, 
__in_chrg=<optimized out>) at /usr/include/c++/4.8.2/bits/stl_set.h:90
    #13 0x0000000000405003 in Mesh::operator= (this=0x7fffffffa8a0, mesh=...)
at mesh.cpp:73
    #14 0x000000000048eb98 in DynamicMesh::reattach_ghost (mpi_comm=1140850688, 
ms=..., cn=..., ms_gh=..., gh=..., cn_gh=..., ale=..., t=4)
at dynamicMesh.cpp:273

В этом случае трассировка #13 соответствует замене std::set.

Моя проблема в том, почему такая ошибка не появляется на первом временном шаге и почему она не появляется на всех процессорах. Более того, эта ошибка иногда возникает в строках, связанных с std::map.

Кроме того, на моем ноутбуке с macOS и Linux код можно успешно запустить; но это не работает на HPC.


c++ mpi assignment-operator
person shidi.yan1992    schedule 02.08.2018    source источник
comment
Поскольку и std::set, и std::map имеют операторы присваивания, я предлагаю вам использовать правило нуля и используйте оператор присваивания, сгенерированный компилятором по умолчанию.   -  person Some programmer dude    schedule 02.08.2018
comment
Я работаю над параллельным кодом И std::set, и std::map не потокобезопасны. Вы синхронизируете доступ к ним? Так как ваш operator= не показывает признаков синхронизации.   -  person Algirdas Preidžius    schedule 02.08.2018
comment
О, и исправьте UB, который у вас есть, ничего не возвращая из операторной функции. И вернуть не постоянную ссылку.   -  person Some programmer dude    schedule 02.08.2018
comment
Проблема в том, что ghostSet.insert(it) должно быть ghostSet.insert(*it). Тем не менее, вы можете заменить все тело операторной функции на {localIndex = mesh.localIndex; ghostSet = mesh.ghostSet; return *this;} или (лучше) вообще не реализовывать оператор (поскольку эффект такой же, как у компилятора по умолчанию).   -  person Peter    schedule 02.08.2018
comment
Ваш оператор присваивания объявлен возвращающим константную ссылку, что совершенно бессмысленно. Более того, в нем нет фактического оператора возврата. Если ваш компилятор не предупредил вас, рассмотрите возможность переключения компиляторов. Замена на временную тоже не имеет особого смысла, можно просто очистить(). Вы можете назначать векторы и карты с помощью оператора =, не нужно писать собственный (неэффективный) цикл копирования. На самом деле ваш класс может отказаться от пользовательского назначения и использовать значение по умолчанию, поскольку там нет неуправляемых указателей. Я сомневаюсь, что это и есть причины крушения, просто хочу иметь в виду.   -  person n. 1.8e9-where's-my-share m.    schedule 02.08.2018
comment
@ Какой-то программист, чувак, спасибо за ответ. Я исправил возврат. Я использовал оператор присваивания по умолчанию для std::set и set::map и все еще не работает; а потом я перешел на эту сложную вещь.   -  person shidi.yan1992    schedule 02.08.2018
comment
@AlgirdasPreidžius, на этих этапах каждый процесс независим, у меня есть petsc для управления глобальной синхронизацией.   -  person shidi.yan1992    schedule 02.08.2018
comment
@н.м. Спасибо за ваш ответ. Я использовал оператор clear() и =, он тоже не работает. И это только часть класса, я должен использовать пользовательский оператор присваивания для других частей.   -  person shidi.yan1992    schedule 02.08.2018
comment
@Peter ghostSet.insert(it) правильно, сбивает с толку it - это значение, а не итератор   -  person Alan Birtles    schedule 02.08.2018
comment
Это вуду, а не разработка. = не сработало, давайте станцуем дождь вокруг этой линии. Нет, тоже не получится.   -  person n. 1.8e9-where's-my-share m.    schedule 02.08.2018
comment
@ shidi.yan1992 shidi.yan1992 проблема не в опубликованном вами коде, что-то еще повреждает набор/карту, сбой происходит в опубликованном вами коде, но причина в другом   -  person Alan Birtles    schedule 02.08.2018
comment
Нет никаких указаний на то, что эта проблема связана с MPI. Может быть, это даже не в коде, который вы предоставили. Подготовьте минимально воспроизводимый пример.   -  person Zulan    schedule 02.08.2018
comment
@AlanBirtles Спасибо за ваш ответ. Посмотрю другие части.   -  person shidi.yan1992    schedule 02.08.2018
comment
@shidi.yan1992 Хотя итераторы std::set или std::map остаются действительными при вставке или перемещении элемента (например, при внутренней перебалансировке дерева), они становятся недействительными при удалении (что, конечно, происходит и при очистке). Поэтому, если вы держите итератор для установки или сопоставления где-либо перед назначением, убедитесь, что вы больше не используете его после этого!   -  person Aconcagua    schedule 02.08.2018

Ответы (1)


arrow_upward
1
arrow_downward

Слишком сложно! Шаг 1: И std::set, и std::map имеют функцию clear, поэтому нет необходимости обмениваться пустыми временными файлами:

/* const*/ Mesh& Mesh::operator=(Mesh const& other)
// why return const? 'this' isn't const either;
// if at all, you only prevent using it directly afterwards:
// Mesh x, y;
// (x = y).someNonConstFunction();
{
    //std::set<size_t>().swap(ghostSet);  ///BUG here
    //std::map<size_t, size_t>().swap(localIndex); /// BUG sometimes here

    localIndex.clear();
    for(auto const &it : other.localIndex)
        localIndex[it.first] = it.second;

    ghostSet.clear();
    for(auto const &it : other.ghostSet)
        ghostSet.insert(it);
}

Изменение порядка очистки выше просто для лучшей иллюстрации шага 2: и std::map, и std::set уже предоставляют операторы присваивания, которые делают именно то, что делают очистка и цикл копирования:

Mesh& Mesh::operator=(Mesh const& other)
{
    //localIndex.clear();
    //for(auto const &it : other.localIndex)
    //    localIndex[it.first] = it.second;
    localIndex = other.localIndex;


    //ghostSet.clear();
    //for(auto const &it : other.ghostSet)
    //    ghostSet.insert(it);
    ghostSet = other.ghostSet;

    // now fixing as well:
    return *this;
}

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

class Mesh
{
    Mesh& Mesh::operator=(Mesh const& other) = default;
};

Я работаю над параллельным кодом [...]

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

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

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

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

person Aconcagua    schedule 02.08.2018
comment
Спасибо за ваше время и подробный ответ. В моем случае мьютекс может не работать. Я использую распределенную память, и каждый процесс независим, каждый процесс имеет свой собственный набор и карту. - person shidi.yan1992; 02.08.2018
comment
Ясно... Тогда проблема не связана с параллелизмом, и начальное предложение вашего вопроса в лучшем случае вводит в заблуждение (не нужно отбрасывать его, но имейте в виду для будущих вопросов: не предоставляйте ненужную информацию). - person Aconcagua; 02.08.2018