LINUX.ORG.RU

Способ на vptr data race

 bad design, ,


0

5

Есть data race прям как в примере тут: https://github.com/google/sanitizers/wiki/ThreadSanitizerPopularDataRaces#dat...

<Для Ъ>:

struct Base {
    virtual ~Base() { 
        unregister(this); // внутри сложный код с синхронизацией
    }

    virtual void someMethod() = 0;
};

struct Derived : public Base {
    ~Derived() { simpleDestruction(); }
    void someMethod() override;
};

Проблема такова, что деструктор и someMethod() могут быть вызваны с разных потоков. А используемые в проекте реализации GCC/Clang меняют vptr при вызове родительских деструкторов. Соотв. если во время синхронизации в ~Base() будет вызван someMethod() получится pure virtual method call. (так как используемые компиляторы GCC).

</Для Ъ>

Было бы дешево и сердито решить проблему переносом синхронизации в деструктор дочернего класса, однако в реальности ситуация такова, что существует дерево из 6 потомков класса Base:

Base_______________
|        \         \
Derived1  Derived2  Derived3
|         |
Derived11 Derived21
|
Derived111
И каждый из потомков может использоваться самостоятельно. Т.е. если реальный тип обьекта Derived111, то синхронизация должна быть в ~Derived111 (и не происходить ни в ~Derived11, ни в Derived1, ни в ~Base). Однако если реальный тип — Derived11, то синхронизация должна происходить только в ~Derived11.

Подскажите пожалуйста как поправить проблему? Может существует какой-нибудь паттерн для такого?

Проблема такова, что деструктор и someMethod() могут быть вызваны с разных потоков.

Дальше можно не читать. У тебя в консерватории что-то не так.

anonymous
()

Положить обьект в shared_ptr. Одна копия у потока который удаляет, одна у потока который вызывает метод. Тогда обьект будет удален только когда оба перестанут им пользоваться.

pftBest ★★★★
()

А как ты шаришь объект и зовёшь у него деструктор? Есди шаришь объект(т.е в обоих потоках клиенты), оборачивай его в shared_ptr. И вообще с чего ты взял что это проблема? Где в стандарте написано что должно быть иное поведение?

lberserq
()
Ответ на: комментарий от pftBest

Одна копия у потока который удаляет, одна у потока который вызывает метод. Тогда обьект будет удален только когда оба перестанут им пользоваться.

Проблема не в том. Проблема в vptr (указатель на vtable): во время деструкции стандарт требует, чтоб вызывались виртуальны методы класса, деструктор которого в процессе. Как именно оно должно быть реализовано — стандарт не сообщает. И фактически все компиляторы реализуют требование через подмену vptr.

А проблема такова, что пока деструктор Base «залипает» на мютексе (ждет пока обьект никто не будет использовать) другой поток может завладеть обьектом Derived и вызвать someMethod(). Из-за того, что vptr был уже подменен в ~Base() будет pure virtual method call.

Друшими словами — в коде нет проблем с вызовом методов удаленного обьекта (присутствует синхронизация). Однако из-за синхронизации есть проблема с vptr. Т.е. shared_ptr не поможет

Не знаю как лучше описать. Но в примере по ссылке как раз хорошо описано.

KennyMinigun ★★★★★
() автор топика
Ответ на: комментарий от lberserq

А как ты шаришь объект и зовёшь у него деструктор?

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

Есди шаришь объект(т.е в обоих потоках клиенты), оборачивай его в shared_ptr.

Почти так и происходит.

И вообще с чего ты взял что это проблема?

Потому, что мне Thread Sanitizer зарапортовал об этом.

Где в стандарте написано что должно быть иное поведение?

Стандарт только требует чтоб в деструкторе вызывались виртуальные методы текущего класса (или его родителей). Как это реализовать — не сообщается. Вот GCC и Clang (да и наверное почти все компиляторы) и подменяют vptr в деструкторе.

KennyMinigun ★★★★★
() автор топика
Ответ на: комментарий от KennyMinigun

А проблема такова, что пока деструктор Base «залипает» на мютексе (ждет пока обьект никто не будет использовать) другой поток может завладеть обьектом Derived и вызвать someMethod(). Из-за того, что vptr был уже подменен в ~Base() будет pure virtual method call.

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

А подсказанный вам уже shared_ptr как раз и является одним из простых способов обеспечения этого режима.

eao197 ★★★★★
()
Ответ на: комментарий от KennyMinigun

Проблема не в том.

Именно в этом, проблема в архитектуре. Я предложил убрать строчку `delete x;` и вместо нее использовать shared_ptr.

Если никто не удаляет обьект напрямую, то и гонки при удалении не будет.

pftBest ★★★★
()

Пока что применил такой полу-костыль (очень упрощенно, для иллюстрации только):

template <class Object>
class Wrapper : public Object {
public:
    template <typename... Args>
    Wrapper(Args&&... args) 
      : Object(std::forward<Args>(args)...) 
    {}

    ~Wrapper() {
        // complex synchronization
        getRegistry().unregister(this); 
    }
}

class Base {
public:
    virtual void someMethod() = 0;
protected:
    // should not be called directly, but via a Wrapper
    Base() = default;
    virtual ~Base() = default;
};

class Derived : public Base {
public:
    void someMethod() override {}
protected:
    Derived() = default;
    ~Derived() = default;
}

void insideCode() {
    getRegistry().put(std::make_shared<Wrapper<Derived>>());

    // ... do smth
}

void otherThread() {
   while (true) {
      for (const auto& watcher : getRegistry())
          watcher->someMethod();

      sleep(5);
   }
}

KennyMinigun ★★★★★
() автор топика
Ответ на: комментарий от pftBest

Именно в этом, проблема в архитектуре.

Тут согласен. См. теги ;-)

Если никто не удаляет обьект напрямую, то и гонки при удалении не будет.

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

Хотя, сейчас попробую оценить насколько заморочно использовать shared_ptr.

KennyMinigun ★★★★★
() автор топика

ООП
C++

Можешь еще попробовать в базовом классе заспавнить тред и повызывать виртуальные функции в нем. А в деструкторе, конечно же, ПОДОЖДАТЬ пока оно джойнится.

Kuzy ★★★
()
Последнее исправление: Kuzy (всего исправлений: 1)

Накалякал такое решение, может где-то ошибся (с memory_order или какую-то ситуацию не предусматрел. Так-же теперь вместо гарантирового исполнения someMethod, он, внезапно, можетбыть не исполнен. DeleteObj и CreateObj должны вызываться из одного и того же потока (не обязательно там-же где и TryMethod) иначе произойдет беда

template <class T> struct SyncObject {
  std::unique_ptr<T> obj;
  std::atomic<bool> deleted;
  std::atomic<bool> is_used;
  void CreateObj() {
    is_used.store(false, std::memory_order_release);
    deleted.store(false, std::memory_order_release);
    obj = std::make_unique<T>();
  }
  void TryMethod() {
    is_used.store(true, std::memory_order_release); //1
    if (!deleted.load(std::memory_order_acquire)) { //2
      obj->someMethod();
    }
    is_used.store(false, std::memory_order_release); //3
  }
  void DeleteObj() {
    deleted.store(true, std::memory_order_release); 
    //После этого момента все попытки войти в 2 провалятся (защита в 1-2)
    while (is_used.load(std::memory_order_acquire)) {
      //Ждем если какой-то объект уже был в 2 до того как мы установили флаг (2-3)
    }
    obj.reset();
  }
};
maxis11
()

Да, никак нормально не сделать в C++

Это плюсопроблемы.

Проблема в том, что логика «закрытия» объекта в C++ совмещена с логикой времени его жизни. У тебя не должно быть такого, что возможность вызвать какой-то виртуальный метод на дохнущем объекте.

В managed языках проблемы этого рода отчасти тоже есть, но там вполне конкретно все разделено. У тебя в принципе ничего не вызывается на дохнущем объекте кроме финализатора. И насчет финализаторов есть куча ограничений. Поэтому, например, в том же C# мире есть паттерн DisposableObject, в котором собственно есть Dispose, который может что-то делать, и вызывается из кода где угодно, но при этом отменяет финализатор, и собственно финализатор, который ну как максимум, освобождает unmanaged ресурсы, и все, а никакой не дай б-г синхронизации, не делает.

Это отчасти решено в COM, опять же тем макаром, что пока у тебя есть хоть где-то ссылка на объект, деструктор не вызовется, но опять же там проблема с reference counting и сложными графами объектов с циклами может быть(банально те же коллбеки, мало кто делает правильно даже в COM).

Короче, я, да и не я один, уже много лет говорю - если у вас сложная логика(что-то сложнее перемножения матриц на стеке), C++ вам использовать нельзя. И всё.

lovesan ★★
()

Подскажите пожалуйста как поправить проблему?

Не использовать вызовы виртуальных функций в конструкторах и деструкторах.

Может существует какой-нибудь паттерн для такого?

К.О. подсказывает: писать Derived11::someMethod() вместо someMethod().

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

asaw ★★★★★
()

проблема в дизайне

3.8 [basic.life]

....

The lifetime of an object of type T ends when: (1.3) — if T is a class type with a non-trivial destructor (12.4), the destructor call starts, or

....

anonymous
()
Ответ на: комментарий от asaw

что деструктор и someMethod() могут быть вызваны с разных потоков

Не использовать вызовы виртуальных функций в конструкторах и деструкторах.

Мимо.

xaizek ★★★★★
()
Ответ на: комментарий от Kuzy

Зачем тут шаред?

А там под капотом еще немного параллелизма, по сути для примера не нужен :)

KennyMinigun ★★★★★
() автор топика
Ответ на: комментарий от asaw

Не использовать вызовы виртуальных функций в конструкторах и деструкторах.

Ты (не)много не понял проблему. Почитай описание по первой ссылке в топике

KennyMinigun ★★★★★
() автор топика
Последнее исправление: KennyMinigun (всего исправлений: 1)
Ответ на: комментарий от asaw

Ты про «получится pure virtual method call» прочитал?

Там мне важнее было «undefined behavior», т.к. только компитятору известно как выглядит великая и могучая vtable.

KennyMinigun ★★★★★
() автор топика
Ответ на: комментарий от asaw

Да, и этого можно добиться не только вызывая виртуальные методы в конструкторе/деструкторе. Но тут UB из-за гонки на vtbl, что там вызовется в результате гонки уже не важно.

xaizek ★★★★★
()
Ответ на: комментарий от KennyMinigun

Ты пишешь на плюсах и до сих пор не знал, когда начинается и когда кончается время жизни объекта?! o_O

anonymous
()
Ответ на: комментарий от xaizek

тут UB из-за гонки на vtbl

Тут UB из-за доступа к объекту вне его времени жизни.

anonymous
()
Ответ на: комментарий от KennyMinigun

Ты (не)много не понял проблему. Почитай описание по первой ссылке в топике

Ну это примерно той же степени лунатизма пример: нужно додуматься спроектировать объект так, чтобы он мог разрушиться только после вызова какой-то функции, а в противном случае будет висеть на мьютексе полуразрушенным. И это ничем не лучше доступа по указателю после delete в однопоточном варианте (о чём уже и написали). Рецепты по ссылке выписаны.

asaw ★★★★★
()
Ответ на: комментарий от anonymous

TryMethod можно использовать из разных тредов. Я про CreateObj/DeleteObj сказал, что там нету никаких проверок на то, что несколько потоков попытаются одновременно создать/удалить объект

maxis11
()
Ответ на: комментарий от anonymous

не, мой метод не работает, там какой-то может флаг вырубить пока другой someMethod() выполняет и полетит все к херам. Его можно только выполнять из какого-то одного потока (другого). А по хорошему надо просто какой нибудь rwlock добавить, тогда снимется ограничение на один поток чтения

maxis11
()
Ответ на: комментарий от maxis11

По-хорошему надо не изобретать велосипед, а использовать shared/weak_ptr.

Лишний раз не-sequential-consistent атомики трогать не стоит.

anonymous
()
Ответ на: комментарий от anonymous

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

anonymous
()
Ответ на: комментарий от anonymous

Чем онанизм с AbstractSingletonProxyFactoryBean лучше онанизма с указателями?

anonymous
()
Ответ на: комментарий от maxis11

мля. эпичное лечение кривых рук инвалидным креслом.

anonymous
()
Ответ на: комментарий от anonymous

Ты пишешь на плюсах и до сих пор не знал, когда начинается и когда кончается время жизни объекта?! o_O

Я пишу на плюсах и до сих пор не знаю плюсов.

KennyMinigun ★★★★★
() автор топика
Ответ на: комментарий от lovesan

Короче, я, да и не я один, уже много лет говорю - если у вас сложная логика(что-то сложнее перемножения матриц на стеке), C++ вам использовать нельзя. И всё.

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

Iron_Bug ★★★★★
()
Ответ на: комментарий от KennyMinigun

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

Iron_Bug ★★★★★
()
Последнее исправление: Iron_Bug (всего исправлений: 1)
Ответ на: комментарий от Iron_Bug

Иметь и применять интеллектуальные способности, в данном случае - не использовать C++

Менее интеллектуальные решения я впрочем тоже предложил.

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

lovesan ★★
()
Ответ на: комментарий от lovesan

Твоё отношение к C++ уже все поняли, успокойся и проходи мимо тегов C++ и пиши на том, что тебе там нравится (какая-нибудь джавка или шарп. В общем, не буду гадать).

Судить о применимости крестов по одному вопросу показатель довольно свежей версии libastral, не более.

У ТСа явно факап в архитектуре (что он признал ещё в тегах). И у него классическое UB в коде (это отсылка к времени жизни обьекта).

А как в архитектуре мешают кресты - ну хз-хз.

zamazan4ik ★★
()
Ответ на: комментарий от zamazan4ik

Слушай, у меня опыта в C++ побольше, чем у некоторых тут опыта в IT вообще.

Но в целом, как бы, это, не могу, «в интернете кто-то не прав».

lovesan ★★
()

Я мало что понял в описании задачи, но суть в том, что когда вызывается деструктор, на объект не должно быть используемых ссылок, т.е. someMethod() не должен вызываться. Так что косяк в архитектуре приложения - сначала все потоки, у которых есть ссылки на объект должны перестать его использовать и только потом у этого объекта можно вызывать деструктор. Для этого, например, можно передавать ссылки на него в виде shared_ptr.

anonymous
()
Ответ на: комментарий от KennyMinigun

А проблема такова, что пока деструктор Base «залипает» на мютексе (ждет пока обьект никто не будет использовать) другой поток может завладеть обьектом Derived и вызвать someMethod().

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

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

anonymous
()
Ответ на: комментарий от lovesan

Иметь и применять интеллектуальные способности, в данном случае - не использовать C++

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

anonymous
()
Ответ на: комментарий от anonymous

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

У C++ нет области применения.

lovesan ★★
()
Ответ на: комментарий от lovesan

Мне, к сожалению, местами тоже приходится плюсы
У C++ нет области применения.

Шиза. Шиза косит стройные ряды лавсанчигов-красавчегов.

eao197 ★★★★★
()
Ответ на: комментарий от Iron_Bug

не надо идти в программисты, если не можешь продумать алгоритм

Надо сразу идти в менеджеры, чтобы рукойводить программистами.

rupert ★★★★★
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.