Thread-safe shared_ptr , Реализация потоко-безопасного SharedPtr

[Cfon]

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

Сообщение отредактировано: Cfon - 20.01.17, 06:26

[Олег М]

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

[Flex Ferrum]

Цитата Олег М @ 20.01.17, 03:52

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

Чтобы решить эту проблему не обязательно выясгять, что я буду потом делать с этим указателем. Есть код и нужно просто прочитать его и сказать – работает он или нет и почему.

Добавлено 20.01.17, 04:05
По поводу XY – на мой взгляд основная проблема в том, что я спрашиваю об одном, а мне пытаются читать лекции совершенно о другом. Насколько я понял всх тут смутило слово sharedptr – мозг тут же отключается от задачи, дальше можно не читать и т.д. Ну, давайте я назову класс как–нибудь по другому – TreadSafePtr, например или как угодно. Тогда может ктонибудь удосужится посмотреть код?

О проблеме XY я заговорил потому, что мне, как человеку, знакомому с этими вещами, не совсем ясно - для каких именно задач ты хочешь применить то, о чём спрашиваешь. При том условии, что обычно работа с такими сущностями ведётся несколько иначе. Тут же ведь ещё в чем фишка: как только ты внятно сформулируешь бизнес-задачу, которую хочешь решить, ты поймёшь - как должен работать твой код. Но, закончим сказку про белого бычка, перейдём к коду.

Предварительный совет номер раз: применять агрессивные средства "ручной" синхронизации типа спинлоков и атомиков нужно тогда, когда код, написанный более традиционно и просто, отлажен и работает. На атомиках и опытные программисты, бывает, спотыкаются. А спинлоки... Лучше брать или готовые (если они прям вот точно нужны), либо не выеживаться и использовать обычные мьютексы.
Предварительный совет номер два: нарисовать на бумажке диаграмму состояний объекта бывает полезно, чтобы понять - в каких случаях и что должно делаться. Связка shared_ptr-weak_ptr - не такая тривиальная, как кажется. Сможете сходу ответить, какой из счётчиков что считает и чьё время жизни контролирует?

Теперь собственно по коду:
1. CRefCounter. У тебя AddRef делается через compare_exchange, а Release, почему-то, нет. Хотя эти операции симметричные и со счетчиком должны работать одинаково. Либо в обоих случаях простой инкремент/декремент атомика, либо в обоих случаях с проверкой.
2. CSharedPtrLock. Идея любого подобного лока - не давать одному потоку выполниться, пока работает другой. То есть, у тебя один поток захватывает лочку, остальные ждут, а потом - ну, уж кто первый успел. У тебя же вызов метода lock просто увеличивает счётчик. А должен блокировать! То есть если один поток вызвал lock с нулевым m_cnt, то все остальные должны честно ждать (на вызове lock), пока первый сделает unlock. Поэтому лочка ничего на самом деле не лечит, а просто считает количество одновременно "заблокированных" объектов. Это, очевидно, не то, что тебе нужно.
3. GetSharedPtrLock. Возвращает статический экземпляр лочки для поинтеров конкретного типа. То есть реализует идею, что все экземпляры CSharedPtr должны друг друга лочить, даже если хранят разные указатели. Это точно то, что тебе нужно?
4. CSharedPtrBase. Идея твоего потокобезопасного смарт-указателя - возможность использовать одну копию SharedPtr из разных потоков. Что будет, если для одного объекта из двух потоков одновременно позвать Detach? Правильно, плохо будет, хотя и не фатально. А вот криво работающая глобальная лочка ни разу не защищает m_cnt от того, что его кто-нибудь не прибьет сбоку.

Продолжение следует.

Сообщение отредактировано: Flex Ferrum - 20.01.17, 08:20

[Олег М]

1. CRefCounter работает с std::atomic, для которого операции инкремент-декремент являются атомарными. AddRef сделан через compare_exchange чтобы отслеживать ноль. В Release этого делать не нужно, достаточно выполнять требование, чтоб Release вызывался только после успешного вызова AddRef
2. CSharedPtrLock нужен исключительно, чтоб предотвратить удаление CSharedRef::m_cnt, в деструкторах SharedPtr и WeakPtr, до тех пор пока кто-то выполняет GetSharedRef. Всё остальное синхронизируется при помощи CSharedRef. По функционалу это дешёвый аналог Read/Write блокировки.
3. Статическим он сделан потому, что я не знаю, как ещё можно синхронизировать указатели на CSharedLock. Блокирует он только удаление, поэтому я решил, что стоимость этого будет невелика.
4. Ну, во-первых, Detach объявлен как protected (по-сути должен быть private, но не суть важно). И он вызывается только в деструкторах и move-конструкторах, которые не могут быть вызваны одновременно в разных по-определению.

[Flex Ferrum]

Цитата Олег М @ 20.01.17, 08:33

1. CRefCounter работает с std::atomic, для которого операции инкремент-декремент являются атомарными. AddRef сделан через compare_exchange чтобы отслеживать ноль. В Release этого делать не нужно, достаточно выполнять требование, чтоб Release вызывался только после успешного вызова AddRef

Можно было бы посмотреть, как это в shared_ptr реализовано. Ну, полезно же. Постфиксный атомарный инкремент и декремент возвращают предыдущее значение переменной (это если верить доке). То есть на release, если в переменной было 1 (то есть постфиксный декремент вернул 1), то можно убивать объект - после декремента будет 0. Кроме того, не в AddRef надо проверять на ноль, а в Release. Ибо именно уменьшение счётчика до 0 разрешает убивать объект. В std::shared_ptr - именно так.

Цитата Олег М @ 20.01.17, 08:33

2. CSharedPtrLock нужен исключительно, чтоб предотвратить удаление CSharedRef::m_cnt, в деструкторах SharedPtr и WeakPtr, до тех пор пока кто-то выполняет GetSharedRef. Всё остальное синхронизируется при помощи CSharedRef. По функционалу это дешёвый аналог Read/Write блокировки.

Я понимаю, зачем он нужен. Просто он не работает так, как запланировано. Он вообще никак не работает - ни как "дешёвый аналог" RWLock, ни как "дешёвый аналог" критической секции. Лучше взять std::mutex и спокойно работать.

Цитата Олег М @ 20.01.17, 08:33

3. Статическим он сделан потому, что я не знаю, как ещё можно синхронизировать указатели на CSharedLock. Блокирует он только удаление, поэтому я решил, что стоимость этого будет невелика.

Для начала определись - у тебя что является разделяемым ресурсом? CSharedCounter или CSharedPtr/CWeakPtr? После этого ты поймёшь, членом какого именно класса должна быть лочка. Глобальная шаренная лочка - плохая, очень плохая идея.

Цитата Олег М @ 20.01.17, 08:33

Ну, во-первых, Detach объявлен как protected (по-сути должен быть private, но не суть важно). И он вызывается только в деструкторах и move-конструкторах, которые не могут быть вызваны одновременно в разных по-определению.

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

Обещанное продолжение.
5. Деструктор CSharedPtr/CWeakPtr. Тут нюанс в том, что Wait на лочке не гарантирует, что никакой поток не "вклинится" между выходом из Wait и вызовом delete.
6. Пока выходит так, что разделяемый ресурс в тебя - это собственно CSharedCounter. То есть работа с одной копией CSharedPtr из разных потоков у тебя защищается исключительно atomic'ом. Как многопоточно (и без лочек) работать с разделяемым shared_counter'ом - можно посмотреть всё в том же <memory> с shared_ptr. Или почитать у Александреску. Особых бенефитов от использования такой версии shared_ptr'а в многопоточной среде я не вижу.

Добавлено 20.01.17, 10:07
И вопрос о том, зачем такой огород городить, остаётся открытым. Ибо принципиально код от shared_ptr'а не отличается (по семантике).

[Олег М]

1. У меня там, в Release, вроде префиксный декремент, return --m_cnt == 0; И он проверяется на ноль. В AddRef проверка на ноль нужна, чтобы не захватывать удаляемый элемент.
2. CSharedPtrLock блокирует delete ref.m_cnt; больше ни для чего он не нужен. Т.е. он ждёт, пока все выйдут из функции GetSharedRef, затем можно смело удалять счётчик. Все, кто войдёт туда позже, получат ref.m_cnt == nullptr.
3. Указатель на CSharedCounter контролирует доступ к указателю на объект. При этом как-то нужно контролировать доступ к указателю на CSharedCounter, это сделано с помощью статического CSharepPtrLock
4. std::atomic<CSharedRef> разве не лочка? И что значит "синхронизировать конструктор и деструктор"? они вроде по-очереди вызываются
5. см. пункт 2. Если вклинится получит nullptr, и всё будет хорошо
6. Нет, разделяемый ресурс - это указатель на объект. Защищается он при помощи счётчика, который должен работать в потоках. Одна из причин, почему это всё защищается при помощи lock-free операций - потому что mutex занимает слишком много памяти. Да и в любом случае - как ты сделаешь общий mutex для всех указателей, и как будешь контролировать доступ к нему?

Добавлено 20.01.17, 10:26
По семантике он и не должен отличаться, отличается по функционалу

Добавлено 20.01.17, 10:50
В этом коде похоже где-то есть ошибка. Я не могу её повторить и не могу понять из-за чего она возникает. Выглядит она примерно так (причём, не уверен): есть SharedPtr и есть WeakPtr, который ссылается на него. После удаления SharedPtr, WeakPtr::lock возвращает не-null, хотя должен

Сообщение отредактировано: Олег М - 20.01.17, 10:52

[Flex Ferrum]

Цитата Олег М @ 20.01.17, 10:25

1. У меня там, в Release, вроде префиксный декремент, return --m_cnt == 0; И он проверяется на ноль. В AddRef проверка на ноль нужна, чтобы не захватывать удаляемый элемент.

Это я к тому, что и в AddRef можно так же делать. К слову, все так и делают.

Цитата Олег М @ 20.01.17, 10:25

2. CSharedPtrLock блокирует delete ref.m_cnt; больше ни для чего он не нужен. Т.е. он ждёт, пока все выйдут из функции GetSharedRef, затем можно смело удалять счётчик. Все, кто войдёт туда позже, получат ref.m_cnt == nullptr.

Но из-за глобальности этой лочки выхода (то есть сброса счётчика до нуля) может и не произойти - им постоянно щёлкают разные копии CSharedPtrBase'а.

Цитата Олег М @ 20.01.17, 10:25

Указатель на CSharedCounter контролирует доступ к указателю на объект. При этом как-то нужно контролировать доступ к указателю на CSharedCounter, это сделано с помощью статического CSharepPtrLock

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

Цитата Олег М @ 20.01.17, 10:25

4. std::atomic<CSharedRef> разве не лочка? И что значит "синхронизировать конструктор и деструктор"? они вроде по-очереди вызываются

Строго говоря, это не лочка. Это просто гарантия, что изменения переменной будут атомарными и видимы на всех процессах/ядрах.

Цитата Олег М @ 20.01.17, 10:25

6. Нет, разделяемый ресурс - это указатель на объект. Защищается он при помощи счётчика, который должен работать в потоках. Одна из причин, почему это всё защищается при помощи lock-free операций - потому что mutex занимает слишком много памяти. Да и в любом случае - как ты сделаешь общий mutex для всех указателей, и как будешь контролировать доступ к нему?

Не надо заниматься предварительной оптимизацией. Тебе нужен потокобезопасный CSharedPtr - это стоит определённых ресурсов. Колхозить собственные lock-объекты в расчёте на то, что они будут работать - довольно специфичный способ развлечения. Попробуй сделат этот код с mutex'ом. С mutex'ом, который находится в экземпляре самого CSharedPtr, и который защищает вызовы Detach, создание копии и т. п. Ты обнаружишь, что код заработает как надо. Ты можешь поместить лочку в CSharedCounter и защищать конкретно этот объект, но... std::shared_ptr уже делает это и делает хорошо.

ЗЫ: И я опять запрашиваю сценарии использования.

[Олег М]

Примерный сценарий (только он выполняется в разных потоках):

CWeakPtr<std::string> sp_weak;
 
{
    auto sp = CSharedPtr<std::string>::Make("!!!!!!!!!!!!!!!!!!!!!!!!!11");
    sp_weak = sp;
}
 
auto sp3 = sp_weak.lock();
//bool(sp3) == true !!!!!!!!!!!!

[Flex Ferrum]

Цитата Олег М @ 20.01.17, 10:57

Примерный сценарий (только он выполняется в разных потоках):

То есть один поток №1 форкает поток №2, в том потоке создаётся CSharedPtr и возвращается в поток №1 посредством WeakPtr? Так? Но... Эм... Как только в потоке №2 SharedPtr сдохнет - в потоке №1 ничего не останется.

[Олег М]

1. Не понял
2. Да, тут есть косяк, в GetSharedRef, он состоит в том, что под блокировкой вызывается ещё одна блокировка ref.m_cnt->m_weaks.AddRef(). Но, скорее всего это ерунда, во всяком случае я не могу пока представить дедлока в этом случае
3. Нет, std::shared_ptr, в gcc, с этим не справляется.
4. Больше ничего и не требуется
5. Ещё раз повторяю - mutex слишком дорогой, sizeof(std::mutex) = 40! Ну, и покажи, как сделать этот код с мьютексами. Лично я что-то не смог придумать ничего адекватного.

Добавлено 20.01.17, 11:15

Цитата Олег М @ 20.01.17, 11:14

То есть один поток №1 форкает поток №2, в том потоке создаётся CSharedPtr и возвращается в поток №1 посредством WeakPtr? Так? Но... Эм... Как только в потоке №2 SharedPtr сдохнет - в потоке №1 ничего не останется.

Проблема как раз выглядит так, что что-то остаётся. Не могу сказать точно, только догадки

Добавлено 20.01.17, 11:18
Если сделаете код, при котором мои классы не работают, буду очень благодарен.

[Flex Ferrum]

Цитата Олег М @ 20.01.17, 11:14

Ну, и покажи, как сделать этот код с мьютексами. Лично я что-то не смог придумать ничего адекватного.

Мне для начала нужно понять сценарии использования конкретно такого варианта кода. Ну нету с ними полной ясности. Приведённый ранее пример я бы вообще иначе делал. Я потому и просил описать решаемую задачу (для которой такой поинтер нужен).

[Олег М]

Как раз тот пример, который с потоками, и есть один из сценариев. Не нужно (хотя, буду весьма признателен, и с удовольствием посмотрю) его делать по другому, иначе это будет другая задача. Проблема в том, что он прекрасно работает. Нужны идеи, в каком случае эти классы не будут работать. У меня их нет,
Добавил модификатор volatile для всех std::atomic. Не знаю, как это может помочь, но хоть с бубнами поплясать

[Flex Ferrum]

Олег М, проблема того примера в том, что он нарушает традиционную логику работы с умными указателями. Я не встречал ещё разработчиков, которые из разных потоков работают с одним экземпляром умного указателя именно вот так. Обычно где-то создаётся объект и копии указателя на него раздаются потокам. Или привлекается внешняя синхронизация для модификации общей для нескольких потоков переменной (в данном случае - умному указателю).

[Олег М]

Нет, логика там другая. Есть счетчик и есть ресурс, доступ к которому осуществляется в зависимости от этого счетчика. Вот и все. Эта задача вполне решаема для потоков.
Чуть позже попытаюсь сформулировать что нибуть поконкретнее, сейчас с планшета пишу, тяжело

[Flex Ferrum]

Цитата Олег М @ 20.01.17, 14:13

Нет, логика там другая.

Ну как же другая? Вот смотрим:

std::shared_ptr<std::string> sp;
std::weak_ptr<std::string> sp2;
 
//CSharedPtr<std::string> sp;
//CWeakPtr<std::string> sp2;
 
volatile bool stop = false;
std::list<std::unique_ptr<std::thread>> threads;
threads.emplace_back(std::make_unique<std::thread>([&sp, &sp2, &stop]()
{

У тебя два указателя во внешнем контексте - sp и sp2. Ты передаёшь их в лямбду по ссылке (то есть не копируешь), а лямбда работает в контексте потока. В итоге у тебя получается, что десяток потоков конкурентно работает с одним и тем же объектом. Без какой-либо синхронизации. Обычно в поток передают копии указателей.

Сообщение отредактировано: Flex Ferrum - 20.01.17, 14:56