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

Неожиданно для себя выяснил, что в gcc реализация классов std::shared_ptr и std::weak_ptr не является потоко-безопасной. В частности конструктор копирования для shared_ptr и функция weak_ptr::lock(). Пришлось попытаться изобрести велосипед написать собственную версию. Прошу критики

#include <atomic>
#include <type_traits>
 
template <typename T> class CSharedPtr;
template <typename T> class CWeakPtr;
 
class CRefCounter
{
public:
 
    bool AddRef()
    {
        for(;;)
        {
            auto cnt = m_cnt.load();
            if (!cnt)
                return false;
 
            if (m_cnt.compare_exchange_weak(cnt, cnt + 1))
                break;
        }
 
        return true;
    }
 
    bool Release() noexcept
    {
        return --m_cnt == 0;
    }
 
 
    explicit operator bool() const noexcept
    {
        return m_cnt != 0;
    }
protected:
    volatile std::atomic<uint32_t> m_cnt{1};
};
 
struct CSharedCounter
{
    CRefCounter m_refs;
    CRefCounter m_weaks;
};
 
struct CSharedRef
{
    CSharedCounter *m_cnt;
    void *m_p;
};
 
class CSharedPtrLock
{
public:
    void lock() noexcept
    {
        ++m_cnt;
    }
 
    void unlock() noexcept
    {
        --m_cnt;
    }
 
    void Wait() const noexcept
    {
        while (m_cnt != 0)
            std::this_thread::yield();
    }
 
protected:
    volatile std::atomic<uintmax_t> m_cnt{0};
};
 
template <typename T>
CSharedPtrLock &GetSharedPtrLock()
{
    static CSharedPtrLock _lock;
    return _lock;
}
 
class CSharedPtrBase
{
public:
    CSharedPtrBase() noexcept
    {
    }
 
protected:
    CSharedPtrBase(CSharedRef ref) noexcept
    : m_ref(ref)
    {
    }
 
    CSharedPtrBase(CSharedPtrBase &&sp) noexcept
    : m_ref(sp.Detach())
    {
    }
 
    void _swap(CSharedPtrBase &sp) noexcept
    {
        m_ref = sp.m_ref.exchange(m_ref);
    }
 
    CSharedRef Detach() noexcept
    {
        return m_ref.exchange({nullptr, nullptr});
    }
 
    template <typename T>
    CSharedRef GetSharedRef() const noexcept
    {
        std::unique_lock<CSharedPtrLock> lock(GetSharedPtrLock<T>());
        auto ref = m_ref.load();
        return ref.m_cnt && ref.m_cnt->m_weaks.AddRef()? ref: CSharedRef{nullptr, nullptr};
    }
 
    volatile std::atomic<CSharedRef> m_ref{{nullptr, nullptr}};
};
 
template <typename T>
class CSharedPtr
: public CSharedPtrBase
{
template <typename T_> friend class CWeakPtr;
public:
    template <typename... TT>
    static CSharedPtr Make(TT&&... args)
    {
        return CSharedPtr(std::make_unique<T>(std::forward<TT>(args)...));
    };
 
    CSharedPtr() noexcept
    {
    }
 
    explicit CSharedPtr(std::unique_ptr<T> &&sp)
    : CSharedPtrBase({sp? new CSharedCounter(): nullptr, sp.get()})
    {
        sp.release();
    }
 
    explicit CSharedPtr(T *p)
    : CSharedPtr(std::unique_ptr<T>(p))
    {
    }
 
    CSharedPtr(const CSharedPtr &sp) noexcept
    : CSharedPtr(sp.GetSharedRef<T>())
    {
    }
 
    CSharedPtr(CSharedPtr &&sp)
    : CSharedPtrBase(sp.Detach())
    {
    }
 
    ~CSharedPtr()
    {
        auto ref = Detach();
        if (!ref.m_cnt)
            return;
 
        if (ref.m_p && ref.m_cnt->m_refs.Release())
            delete reinterpret_cast<T *>(ref.m_p);
 
        if (ref.m_cnt->m_weaks.Release())
        {
            GetSharedPtrLock<T>().Wait();
            delete ref.m_cnt;
        }
    }
 
    T *get() noexcept
    {
        return reinterpret_cast<T *>(m_ref.load().m_p);
    }
 
    void reset() noexcept
    {
        CSharedPtr()._swap(*this);
    }
 
    template <typename T_>
    void reset(T_&& p)
    {
        CSharedPtr(std::forward<T_>(p))._swap(*this);
    }
 
    explicit operator bool() const noexcept
    {
        return this->m_ref.load().m_p != nullptr;
    }
 
    T *operator->() noexcept
    {
        return get();
    }
 
    T &operator *() noexcept
    {
        return *get();
    }
 
    CSharedPtr &operator =(const CSharedPtr &sp)
    {
        if (&sp != this)
            CSharedPtr(sp)._swap(*this);
 
        return *this;
    }
 
    CSharedPtr<T> &operator =(CSharedPtr &&sp)
    {
        _swap(sp);
        return *this;
    }
 
protected:
    CSharedPtr(CSharedRef ref) noexcept
    : CSharedPtrBase({ref.m_cnt, ref.m_cnt && ref.m_p && ref.m_cnt->m_refs.AddRef()? ref.m_p: nullptr})
    {
    }
};
 
template <typename T>
class CWeakPtr
: public CSharedPtrBase
{
public:
    CWeakPtr() noexcept
    {
    }
 
    CWeakPtr(CSharedPtr<T> sp) noexcept
    : CSharedPtrBase(sp.GetSharedRef<T>())
    {
    }
 
    CWeakPtr(const CWeakPtr &sp) noexcept
    : CSharedPtrBase(sp.GetSharedRef<T>())
    {
    }
 
    CWeakPtr(CWeakPtr &&sp) noexcept
    : CSharedPtrBase(sp.Detach())
    {
    }
 
    ~CWeakPtr()
    {
        auto ref = Detach();
        if (ref.m_cnt && ref.m_cnt->m_weaks.Release())
        {
            GetSharedPtrLock<T>().Wait();
            delete ref.m_cnt;
        }
    }
 
    CSharedPtr<T> lock() const noexcept
    {
        return CSharedPtr<T>(GetSharedRef<T>());
    }
 
    void reset() noexcept
    {
        CWeakPtr()._swap(*this);
    }
 
    template <typename T_>
    CWeakPtr &operator =(CSharedPtr<T_> sp)
    {
        CWeakPtr(std::move(sp))._swap(*this);
        return *this;
    }
 
    CWeakPtr &operator =(const CWeakPtr &sp)
    {
        if (&sp != this)
            CWeakPtr(sp)._swap(*this);
 
        return *this;
    }
};

Прикреплённый файлSharedPtr.h (4,43 Кбайт, скачиваний: 145)

Сообщение отредактировано: Олег М - 23.01.17, 05:57

синхронизация на что дана?

В смысле? Синхронизацию чего с чем ты имеешь ввиду?

Цитата Олег М @ 16.01.17, 13:49

Пришлось попытаться изобрести велосипед написать собственную версию.

Сразу и не разберёшься - много текста.
На словах скажи, как синхронизируется инкремент/декремент ссылок
на обьект при работе SharedPtr-ми из разных потоков.

Цитата Олег М @ 16.01.17, 17:38

Ну, на словах - увеличиваешь счётчик, если он не равен нулю, CRefCouner::AddRef(). Этот ответ требовался?

Нет. Как синхронизируется...
Из одного потока производится инкремент и вдруг.. из другого
потока декремент.

На словах я это и так знаю. Мне хочется понять, почему мой код не работает. Если не работает.

Добавлено 16.01.17, 17:56

Цитата ЫукпШ @ 16.01.17, 17:40

Цитата Олег М @ 16.01.17, 17:38

Ну, на словах - увеличиваешь счётчик, если он не равен нулю, CRefCouner::AddRef(). Этот ответ требовался?

Нет. Как синхронизируется...
Из одного потока производится инкремент и вдруг.. из другого
потока декремент.

Насчет "вдруг" можно поподробнее, в каком случае это может произойти, и к чему приведет?

Цитата Олег М @ 16.01.17, 17:42

Насчет "вдруг" можно поподробнее, в каком случае это может произойти, и к чему приведет?

"Вдруг" - это реальная работа из разных потоков.

Все действия алгоритма по инкременту/декременту и принятия решения об уничтожении
объекта не являются атомарными. А значит требуют синхронизации при работе из разных
потоков.

1. Возможна такая коллизия - SP_n1 сделал декремент,
обнаружил, что счётчик стал равен 0 и решил уничтожить объект.
Как никому уже не нужный, но не успел это сделать.
В этот момент его грубо прервали.
Из другого потока наоборот - SP_n2 счётчик увеличил, оставаясь
в полной уверенности, что объект существует.
Далее продолжил работу SP_n1 и уничтожил объект.

2. Коллизия по-проще - допустим, сама операция инкремента/декремента
не атомарна. Значит, любая из них может быть прервана не полностю завершённой,
другой такой-же операцией из другого потока. С непредсказуемыми последствиями.

3. я не разбирался точно, как там у тебя всё устроено.
Думаю, что вместе с получением самого первого адреса на объект, в памяти создаётся структура,
содержащая счётчик ссылок и указатель на объект. Адрес на эту структуру получают все SP, работающие
с одним и тем же объектом. И эта структура в памяти должна уничтожаться
вместе с объектом. И все эти действия тоже должны быть сделаны "потокобезопасными".

Сообщение отредактировано: ЫукпШ - 16.01.17, 20:47

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

Читаем документацию на shared_ptr:

Цитата

All member functions (including copy constructor and copy assignment) can be called by multiple threads on different instances of shared_ptr without additional synchronization even if these instances are copies and share ownership of the same object. If multiple threads of execution access the same shared_ptr without synchronization and any of those accesses uses a non-const member function of shared_ptr then a data race will occur; the shared_ptr overloads of atomic functions can be used to prevent the data race.

http://en.cppreference.com/w/cpp/memory/shared_ptr

Из неё следует, что при работе с std::shared_ptr защищать необходимо лишь многопоточные операции с одной копией shared_ptr. Если два потока работают с разными копиями указателя на один объект - всё будет корректно. Следовательно, городить огород со своими велосипедами особого смысла нет. Копирование/присваивание/перенос shared_ptr можно делать под внешней лочкой (если она вообще необходима). Но в качестве практики свой "умный указатель", конечно, написать полезно.

Проблема в одновременном вызове конструктора копирования и функции release. Это не работает, в реализации gcc

Сообщение отредактировано: Олег М - 17.01.17, 13:14

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

Проблема в одновременном вызове конструктора копирования и функции release. Это не работает, в реализации gcc

Если у одной и той же копии shared_ptr - то и не должно работать.

Очевидно, что у разных. Надо постараться, чтоб у одного объекта вызвать одновременно конструктор и метод. Вообще - в одном потоке вызываешь weak_ptr::lock, в другом - shared_ptr::release, и всё, получаешь segmentation fault

Цитата Олег М @ 17.01.17, 13:23

shared_ptr::release

Эм. У shared_ptr есть release? Он вроде только у unique_ptr.

Цитата

и всё, получаешь segmentation fault

Приведи код.

Цитата D_KEY @ 17.01.17, 13:28

Цитата Олег М @ 17.01.17, 13:23

shared_ptr::release

Эм. У shared_ptr есть release? Он вроде только у unique_ptr.

Цитата

и всё, получаешь segmentation fault

Приведи код.

Да, ошибся - shared_ptr::reset
Код сейчас сделаю

Добавлено 17.01.17, 14:11
Это код который не работает на std::shared_ptr и работает на моих классах

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]()
{
    while(!stop)
    {
        sp.reset(new std::string("!!!!!!!!!!!!!!!!!!!!!!!!!!!!11111111111111111111111111111111111111"));
        sp2 = sp;
    }
}));
 
threads.emplace_back(std::make_unique<std::thread>([&sp2, &stop]()
{
    int i = 0;
    while(!stop)
    {
        auto sp = sp2.lock();
        if (sp)
            std::cout << (++i) << ", " << *sp << std::endl;
 
        std::this_thread::sleep_for(10ms);
    }
}));
 
WaitStop();
stop = true;
 
for (auto &item: threads)
    item->join();

Сообщение отредактировано: Олег М - 17.01.17, 14:11

Но так и не должен этот код работать - все твои потоки конкурентно долбятся к двум умным указателям, а не со своими собственными копиями работают. Как раз про такой кейс в доке и сказано, что data race будет.

Собственно причин, почему этот код не должен работать я не вижу. С моими классами вроде работает

Добавлено 17.01.17, 14:55
Кроме того в Microsoft c++ тоже должен работать, но тут врать не буду, не проверял

Цитата Олег М @ 16.01.17, 13:49

Прошу критики

Код не смотрел, но я против! Шютка...

Но в каждой шутке - есть доля шутки. Самый главный вопрос можно сформулировать пока двумя тезисами:

1) Многопоточное приложение наибоее эффективно, если его потоки не простаивают
2) Многопоточное приложение наибоее эффективно, если его потоки тратят на синхронизацию минимум рабочего времени

Хотя первый тезис - сиречь, в постановке вопроса данного топика, можно опустить. Т.к., имея "низкоуровневый" механизм обеспечения многопоточности - мы его ну просто обязаны пользовать на право и на лево. А это лютые, бешеные "тонны" синхронизации!

Конечно, возникнет, "практически законный" вопрос, типа а если очень надо?
Ответ есть: нужно пользовать механизмы обеспечения "параллелизма" более высокого порядка.

Пока расписывать не буду. Соберусь с мыслями и забабахаю тему-конкурс. А пока ... типа "напочитать" тема - тут (если быть точнее 15.2.1).

Дело понятное, мой ответ в стиле "имхо" - ошибаться право имею!

Выяснилось, что эти мои SharedPtr/WeakPtr не работают. Где-то есть косяк. Понять бы где

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

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

Цитата Олег М @ 17.01.17, 14:53

Собственно причин, почему этот код не должен работать я не вижу.

Всё очень просто. Есть объект, на который указывает умный указатель. А есть умный указатель, который указывает на объект. Так вот, объект (указываемый) должен быть один. А вот копий указателей на него - много. Щёлкать ссылками на указываемый объект из разных потоков - не проблема. Это прекрасно можно. Но вот работать из разных потоков с одним экземпляром указывающего объекта (умного указателя) - нельзя. Нужно снять с него копию, передать в другой поток и с этой копией в том потоке работать. Твой же код в нескольких потоках работает именно с одной копией умного указателя (а не указываемого объекта), что прямо запрещено стандартом.

Вот операция "снять копию" и должна быть потоко–безопасной. И в стандарте вроде написано, что это запланировано на будущее. Это раз. А во вторых – ну снимешь ты копию weak_ptr, ну передашь ее в другой поток, и что? Как ты из него shared-ptr планируешь создавать, возвращаться в первый поток?

Цитата Олег М @ 17.01.17, 19:40

Вот операция "снять копию" и должна быть потоко–безопасной.

Не должна. Судя по всему, ты как-то странно предполагаешь shared pointer'ы использовать. Традиционный подход такой: сколько точек "хранения" указателя - столько копий shared_ptr'а. И таково значение счётчика ссылок. Когда последнему пользователю указателя объект перестаёт быть нужным - удаляется последняя копия shared_ptr'а и объект разрушается. Создать указываемый объект в одном потоке и передать его в другой, просто расшарив между потоками одну копию shared_ptr'а? Это что-то оригинальное. Тут явная синхронизация нужна, потому что контейнер (shared_ptr) становится разделяемым ресурсом. Есть, впрочем, ещё один вариант - использовать future's и std::async. Ну, это если по-правильному.

Ты забываешь про weakptr. Это во первых. Во вторых, когда ты создаешь копию shardptr, ты по сути блокируешь ресурс от удаления. Сам бог велел использовать это свойство для синхронизации доступа к ресурсу.

Цитата Олег М @ 17.01.17, 18:20

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

Даже не знаю.
Просто для интереса - я попробовал оба варианта.
С обычными указателями получилось даже проще.

Было так:
1. Некий поток получает данные.
2. Получим память для них, если нет свободного реcурса в обратной очереди.
3. указатель - в очередь. Потокобезопасную.
При закрытии приложения, все данные, чей указатель в очереди, ликвидирует сама очередь.
4. Посылаем уведомление "данные получены"
5. Другой поток получае уведомление.
6. Получает указатель на данные из очереди.
7. Строит графики в памяти.
8. Если данные не нужны, - указатель в обратную очередь.

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

Можно использовать очередь обычных указателей, можно SP. С обычными по-проще будет.

Сообщение отредактировано: ЫукпШ - 17.01.17, 20:31

Цитата Олег М @ 17.01.17, 19:57

Ты забываешь про weakptr. Это во первых. Во вторых, когда ты создаешь копию shardptr, ты по сути блокируешь ресурс от удаления. Сам бог велел использовать это свойство для синхронизации доступа к ресурсу.

Блокировать ресурс от удаления - это не синхронизировать многопоточный доступ к ресурсу. Это совершенно разные вещи. weak_ptr нужен для того, чтобы не держать взаимные ссылки объектов друг на друга (буде такое случиться). Синхронизацию на модифицирующий доступ к shared_ptr это всё равно не отменяет.

Ладно. Перефразирую вопрос –нужен потоко–безопасный sharerptr. Как его реализовать, конструктор копирования, reset и weakptr;;lock?

Ну так что, никто не попытается решить задачу? Хотя бы так, для общего развития? А то, судя по написанному, совсем плохая картина, в области многопоточного программирования.

Олег М, дабы избежать проблемы XY, давай начнём с другого: для решения какой задачи тебе нужен потоко-безопасный смарт-поинтер?

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

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

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

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

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

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

Цитата Олег М @ 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-конструкторах, которые не могут быть вызваны одновременно в разных по-определению.

Цитата Олег М @ 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

Цитата Олег М @ 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 !!!!!!!!!!!!

Цитата Олег М @ 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
Если сделаете код, при котором мои классы не работают, буду очень благодарен.

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

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

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

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

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

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

Цитата Олег М @ 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

Нет, не так. Здесь первый поток ресетит shared_ptr, sp, и присваивает его weak_ptr'у, sp2. Второй поток вызывает weak_ptr::lock, те создаёт новый shared_ptr, и, если он не null, обращается к его содержимому. Для std::shared_ptr это не работает, для CSharedPtr всё нормально

Олег М, вот какая проблема с многопотоковостью SP.
1. Несколько экземпляров SP указывают на объект выбранного типа.
2. Другие несколько экземпляров - на другой объект этого-же типа.
3. SP - это шаблонный класс. Значит, возможно наличие групп SP,
указывающих на объекты любых других типов.

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

Цитата Олег М @ 20.01.17, 15:17

Нет, не так.

Смотри.
Строчка 9:

Цитата Олег М @ 17.01.17, 13:40

threads.emplace_back(std::make_unique<std::thread>([&sp, &sp2, &stop]()

- ты передаёшь в поток оба указателя по ссылке.

Строчка 13:

Цитата Олег М @ 17.01.17, 13:40

sp.reset(new std::string("!!!!!!!!!!!!!!!!!!!!!!!!!!!!11111111111111111111111111111111111111"));

Ты в нескольких потока одновременно модифицируешь один и тот же shared_ptr. То есть незащищённый конкурентный доступ к внешнему ресурсу.

Строчка 14:

Цитата Олег М @ 17.01.17, 13:40

sp2 = sp;

Ты так же конкурентно (как и в предыдущей строке) модифицируешь разделяемый weak_ptr. Один и тот же во всех потоках.

Строчка 18:

Цитата Олег М @ 17.01.17, 13:40

threads.emplace_back(std::make_unique<std::thread>([&sp2, &stop]()

Передаёшь ещё в один поток ссылку на внешний weak_ptr.

Строчка 18:

Цитата Олег М @ 17.01.17, 13:40

auto sp = sp2.lock();

Пытаешься залочить внешний weak_ptr, который активно (без синхронизации) меняется из соседнего потока.
Ты, конечно, можешь ожидать, что будет создана отдельная копия shared_ptr'а на какой-то указатель, но если повезёт, твоя программа свалится с внятной диагностикой. Ибо тут самый что ни наесть типовой data race. Если не повезёт - отправится SMS на адрес Трампа и он нажмёт на красную кнопку в своём ядерном чемоданчике. Undefined Behavior - он такой.

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

Имеешь ввиду template <typename T> CSharedPtrLock &GetSharedPtrLock() и, соответственно, CSharedPtrBase::GetSharedRef()?

Добавлено 20.01.17, 15:42
Flex Ferrum

Цитата Flex Ferrum @ 20.01.17, 15:34

Ты, конечно, можешь ожидать, что будет создана отдельная копия shared_ptr'а на какой-то указатель, но если повезёт, твоя программа свалится с внятной диагностикой. Ибо тут самый что ни наесть типовой data race. Если не повезёт - отправится SMS на адрес Трампа и он нажмёт на красную кнопку в своём ядерном чемоданчике. Undefined Behavior - он такой.

Ну, во-первых не сваливается, работает. Во-вторых как раз эти функции и являются потоко-безопасными. Я там не ожидаю, я точно знаю, что будет создано return CSharedPtr<T>(GetSharedRef<T>());

Добавлено 20.01.17, 15:43
ЫукпШ Имеешь ввиду template <typename T> CSharedPtrLock &GetSharedPtrLock() и, соответственно, CSharedPtrBase::GetSharedRef()?
Очень похоже на адский косяк, сижу, думаю

Цитата Олег М @ 20.01.17, 15:38

Ну, во-первых не сваливается, работает.

Я говорю про работу с shared_ptr. Как аргумент к тому, что конкретно этот пример разделяемым ресурсом считает сам указатель (CSharedPtr/CWeakPtr в твоей терминологии), а не CSharedPtrBase.

Цитата Олег М @ 20.01.17, 15:38

Пытаешься залочить внешний weak_ptr, который активно (без синхронизации) меняется из соседнего потока.

Вообще, там вроде всё синхронизируется, в конструкторах/деструкторах

Цитата Олег М @ 20.01.17, 15:47

Вообще, там вроде всё синхронизируется, в конструкторах/деструкторах

В стандартном? Нет. Не лочится. И не должно - это не требуется стандартом. А работа с атомиками (как я уже говорил) - это не локи. Это просто работа с атомиками.

Цитата Олег М @ 20.01.17, 15:47

Я говорю про работу с shared_ptr. Как аргумент к тому, что конкретно этот пример разделяемым ресурсом считает сам указатель (CSharedPtr/CWeakPtr в твоей терминологии), а не CSharedPtrBase.

Доступ к самому указателю синхронизируется при помощи счётчика CSharedCounter. Что именно неправильно, и как должно быть?

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

Цитата Flex Ferrum @ 20.01.17, 15:50

В стандартном? Нет. Не лочится. И не должно - это не требуется стандартом. А работа с атомиками (как я уже говорил) - это не локи. Это просто работа с атомиками.

В стандартном да, не работает. Я где-то видел, что они планируют, сходу не могу найти где. У меня вроде работает. Работа с атомиками - это lock-free блокировка, чем то-хуже, чем-то лучше мьютексов.

Цитата Олег М @ 20.01.17, 15:51

Доступ к самому указателю синхронизируется при помощи счётчика CSharedCounter. Что именно неправильно, и как должно быть?

Ещё раз: я говорю про работу этого кода с std::shared_ptr/std::weak_ptr.

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

Цитата Олег М @ 20.01.17, 15:51

В стандартном да, не работает. Я где-то видел, что они планируют, сходу не могу найти где. У меня вроде работает. Работа с атомиками - это lock-free блокировка, чем то-хуже, чем-то лучше мьютексов.

Работа с атомиками - это работа с атомиками. То есть работа с памятью с учётом особенностей многопоточной и многопроцессорной среды. Но не более того. Рассматривать это как блокировку (или варианты блокировки) - это обманывать самого себя.

Цитата Flex Ferrum @ 20.01.17, 15:54

Ещё раз: я говорю про работу этого кода с std::shared_ptr/std::weak_ptr.

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

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

И?

Я до сих пор не догоняю, какая именно реальная задача решается таким вот сценарием:

Цитата Олег М @ 20.01.17, 15:17

Здесь первый поток ресетит shared_ptr, sp, и присваивает его weak_ptr'у, sp2. Второй поток вызывает weak_ptr::lock, те создаёт новый shared_ptr, и, если он не null, обращается к его содержимому. Для std::shared_ptr это не работает, для CSharedPtr всё нормально

Цитата Flex Ferrum @ 20.01.17, 15:54

Работа с атомиками - это работа с атомиками. То есть работа с памятью с учётом особенностей многопоточной и многопроцессорной среды. Но не более того. Рассматривать это как блокировку (или варианты блокировки) - это обманывать самого себя.

Ну почему обманывать, у lock-free алгоритмов есть масса преимуществ, особенно на многопроцессорных серверах. Есть и свои недостатки. Я в них не особо разбираюсь, поэтому в данном случае мне больше нравится, что они занимают много меньше памяти - 8 байт против 40 для мьютекса

Добавлено 20.01.17, 16:04
Реальная задача чуть позже. Надо как-то сформулировать. Запустил-таки всю эту бадягу на боевом сервере, наблюдаю, не могу толком ни на чём другом сосредоточиться

Цитата Олег М @ 20.01.17, 16:02

Ну почему обманывать, у lock-free алгоритмов есть масса преимуществ, особенно на многопроцессорных серверах.

lock-free-алгоритмы - это одно из средств применения атомиков. Причём весьма специфичная. Средствами синхронизации атомики от этого не становятся.

Почему? Есть возможность сделать так, чтоб один поток, подождал пока другой поток не освободил ресурс. Это ли не синхронизация?

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

Почему? Есть возможность сделать так, чтоб один поток, подождал пока другой поток не освободил ресурс. Это ли не синхронизация?

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

Убрал template c GetSharedPtrLock(). Теперь они все будут лочиться одним счётчиком. Плохо конечно, но не смертельно. Понять, это ли было причиной ошибки на сервере я не смог, но в случае с потоками лучше перебдеть

Добавлено 20.01.17, 16:53

Цитата Flex Ferrum @ 20.01.17, 16:37

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

А разве не именно такие алгоритмы называются lock-free? Которые блокируют ресурс не усыпляя потока?

Цитата Олег М @ 20.01.17, 16:40

А разве не именно такие алгоритмы называются lock-free? Которые блокируют ресурс не усыпляя потока?

Если я всё правильно понимаю, то нет:

Цитата

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

Добавлено 20.01.17, 17:09
В вики на этот счёт всё хорошо расписано.

Цитата Flex Ferrum @ 20.01.17, 17:06

Если я всё правильно понимаю, то нет:
Цитата

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

Цитата Олег М @ 20.01.17, 17:18

Моё понимание таково - усыплять поток, потом будить - это дорого. Для многопроцессорных систем дешевле, чтоб этот поток покрутился в цикле, пока другой поток, который как правило крутится на другом другом процессоре, освободит ресурс.

Нет. Самый дорогой ресурс - процессорное время.
Если потоку не нужно работать, пусть остановится.

Цитата ЫукпШ @ 20.01.17, 17:22

Нет. Самый дорогой ресурс - процессорное время.
Если потоку не нужно работать, пусть остановится.

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

Цитата Олег М @ 20.01.17, 17:18

Цитата Flex Ferrum @ 20.01.17, 17:06

Если я всё правильно понимаю, то нет:
Цитата

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

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

Дорого контекстами щелкать - прыгать от одного потока к другому и обратно (т. н. "дребезг") - вот это да, действительно дорого. Поток должен стараться с максимальной пользой утилизировать свои кванты. Этим и хороши lock-free алгоритмы (при грамотно использовании) - на критических путях нету взаимных блокировок и простоев. Но и они не панацея.

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

А чем самопальные спинлоки отличаются о не-самопальных.

Добавлено 20.01.17, 17:41

Цитата Flex Ferrum @ 20.01.17, 17:32

Дорого контекстами щелкать - прыгать от одного потока к другому и обратно (т. н. "дребезг") - вот это да, действительно дорого. Поток должен стараться с максимальной пользой утилизировать свои кранты. Этим и хороши lock-free алгоритмы (при грамотно использовании) - на критических путях нету взаимных блокировок и простоев. Но и они не панацея.

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

Цитата Олег М @ 20.01.17, 17:37

А чем самопальные спинлоки отличаются о не-самопальных.

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

Цитата Flex Ferrum @ 20.01.17, 17:42

Тем, что могут не учитывать особенности работы системы, быть написанными топорно.

А как цикл с compare_exchange может не учитывать особенности работы системы? Или учитывать?

Цитата Flex Ferrum @ 20.01.17, 10:52

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

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

В смысле, ты имеешь ввиду, почему я не делаю AddRef () {return ++m_cnt != 1;}? Потому что так нельзя. Если счётчик равен нулю, то его нельзя увеличивать, иначе когда несколько потоков будут одновременно вызывать AddRef(), только одному вернётся false, остальным - true и указатель на объект который был удалён.

Итак - зачем мне это нужно?
1. Тренировка мозгов. Когда работаешь с потоками ошибку сделать легко, но найти её можно только путём пристального разглядывания и анализа исходного кода. Для этого нужно всегда быть в форме и помнить о нюансах и правилах. Lock-free алгоритмы - хорошая тренировка.

Добавлено 22.01.17, 07:22
2. Повторное использования кода. Когда тебе постоянно приходится блокировать экземпляр какого-то класса, придумывать названия для мьютексов, следить, чтобы они блокировались-разблокировались вовремя и тд и тп, поневоле задумаешься - а не сделать ли этот класс потоко-безопасным? Потратить один раз время и забыть об этих проблемах. C классами типа контейнеров, строк и т.д. это делать тяжеловато, да и особо не нужно - они блокируются относительно редко и по-разному. Но указатели - другое дело, они простые, и они используются часто.

Добавлено 22.01.17, 07:54
3. Минимизация времени блокировки. Не думаю (вернее, не знаю как), что можно заблокировать shared_ptr на меньшее время, причём независимо от других указателей. Например есть большое количество объектов, которые создаются/удаляются в одних потоках, обрабатываются в других. Для решения этой задачи я пользуюсь следующим методом:
- В обработчике хранится список list<weak_ptr> и мьютекс
- Когда мне надо что-то с ним сделать, я блокирую этот мьютекс, переношу список в локальную переменную при помощи move-конструктора, разблокирую
- Пробегаюсь по локальному списку, если weak_ptr::lock возвращает null, удаляю из локального списка, иначе вызываю какую-нибудь функцию
- Снова блокирую список, делаю list::splice, т.е. возвращаю не удалённые объекты обратно
Все операции копеечные и никаких дедлоков

смотрел смотрел твой код.. сложно не разобраться добавь комментов плз

Цитата Олег М @ 22.01.17, 07:13

3. Минимизация времени блокировки.

Критическая секция - вероятно, самый быстрый вариант.
---
Тренировка - это отлично. А результат достигнут ?

Сообщение отредактировано: ЫукпШ - 22.01.17, 13:25

Цитата ЫукпШ @ 22.01.17, 13:24

Критическая секция - вероятно, самый быстрый вариант.
---
Тренировка - это отлично. А результат достигнут ?

Тут спорный вариант, но примерно такой же, но, в любом случае занимает больше памяти.
Результат чего? Код вроде работает, пока. Завтра вечером можно будет сказать более определённо. В выходные коннекто мало.

Добавлено 22.01.17, 13:44

Цитата Cfon @ 22.01.17, 12:58

смотрел смотрел твой код.. сложно не разобраться добавь комментов плз

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

Цитата Олег М @ 22.01.17, 13:39

Результат чего? Код вроде работает, пока. Завтра вечером можно будет сказать более определённо.

Вот посмотри тут.
Ты пишешь:

        void lock() noexcept
    {
        ++m_cnt;
    }

Будет ли этот инкремент - потокобезопасным ?
Чисто случайно, я посмотрел ассемблерный текст одного из скомпилированных модулей.
И вот что получилось - вот оно:

//..
 ++Count;
//..

стало:

; 191  :  ++Count;
 
  0003e a1 00 00 00 00   mov     eax, DWORD PTR ?Count@@3HA ; Count
  00043 40       inc     eax
  00044 a3 00 00 00 00   mov     DWORD PTR ?Count@@3HA, eax ; Count

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

            inc DWORD PTR Count

Сообщение отредактировано: ЫукпШ - 22.01.17, 22:09

Цитата ЫукпШ @ 22.01.17, 22:07

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

Всё правильно, операции ++, -- и т.д. не являются атомарными для базовых типов. Для этого следует использовать interlocked-функции. Либо класс std::atomic, как сделано у меня. Тогда всё будет хорошо. И не забывать про модификатор volatile

Цитата ЫукпШ @ 22.01.17, 22:07

Хотя казалось что будет что-то вроде (но увы..):

            inc DWORD PTR Count

Наверное, чуть правильнее так:

           lock inc DWORD PTR Count

Сервер проработал несколько дней, поэтому можно сделать предварительный вывод, что код рабочий. Хотя 100-процентной уверенности так и нет. Жду, что кто-нибудь даст мне идеи, когда этот код не работает. Вообще, там есть есть две вещи, которые мне не нравились с самого начала:
- совершенно отвратительный статический CSharedPtrLock. Но ничего более умного я не смог придумать, чтоб ограничить доступ к указателю на счётчик;
- функция CSharedPtr::get(). Хотелось бы обращаться к указателю без использования atomic-операции.

нашел статейку от Саттера:
https://www.infoq.com/news/2014/10/cpp-lock-free-programming
отсюда надо плясать?

Сообщение отредактировано: Cfon - 23.01.17, 17:12

Не знаю даже, статей полно, причём конкретно эта мне кажется какой-то мутной. В частности реализация single-linked list при помощи shared_ptr.
Там одна операция auto p = make_shared<Node>(); уже не является lock-free

Саттер налажал?

Мне, кстати, вообще не даёт создать std::atomic<std::shared_ptr<std::string>>, говорит error: static assertion failed: std::atomic requires a trivially copyable type

И не должен.

Не знаю даже. Основная проблема при реализации single-linked - это то что ты не можешь поменять значение указателя при помощи compare_exchange. Нужно менять значение пары {счётчик, указател}. Соответственно head представляет из себя структуру {size_t, void *}. shared_ptr же - это два указателя, как им можно заменить счётчик, я не знаю.

Цитата Славян @ 23.01.17, 14:25

Цитата ЫукпШ @ 22.01.17, 22:07

Хотя казалось что будет что-то вроде (но увы..):

            inc DWORD PTR Count

Наверное, чуть правильнее так:

           lock inc DWORD PTR Count

Теоретически "да", но не факт.
Нужно точно знать аппаратое устройство машины, а арбитр шины
теперь наверняка внутри процессора.
"Lock" - это всего-лишь пин процессора, торчащий провод.
Сигнал принимает активное значение на время команды, снабжённой
одноимённым преффиксом.
Куда он запаян разработчиком заранее не всегда извесно.
Например, у IBMPC-AT он просто висел в воздухе.

Добавлено 23.01.17, 18:35

Цитата Олег М @ 23.01.17, 05:55

Для этого следует использовать interlocked-функции.

Тут есть рекомендации. Глава 8

Цитата

...
Применяйте эту методику с крайней осторожностью, потому что процессорное время
при спин-блокировке тратится впустую Процессору приходится постоянно сравнивать
два значения, пока одно из них не будет "волшебным образом» изменено другим потоком.
Учтите - этот код подразумевает, что все потоки, использующие спин блокировку,
имеют одинаковый уровень приоритета. К тому же. Вам, наверное, при дется отключить
динамическое повышение приоритета этих потоков (вызовом SetProcessPriorityBoost или SetThreadPriorityBoost).
...
Спин-блокировка предполагает, что защищенный ресурс не бывает занят надолго.
И тогда эффективнее делать так: выполнять цикл, переходить в режим ядра и ждать.
Многие разработчики повторяют цикл некоторое число раз (скажем, 4000) и, если ресурс к тому
времени не освободился, переводят поток в режим ядра, где он спит, ожидая освобождения ресурса
(и не расходуя процессорное время). По такой схеме реализуются критические секции (critical sections).
...

Сообщение отредактировано: ЫукпШ - 23.01.17, 18:36

ЫукпШ Собственно, я её, методику, и применяю с крайней осторожность, ресурс вроде не блокируется надолго.

Цитата Олег М @ 20.01.17, 17:46

А как цикл с compare_exchange может не учитывать особенности работы системы? Или учитывать?

Цикл с compare_exchange - это цикл, для которого компилятор сгенерирует какой-то код. А как долго допустимо его крутить? Может есть что-то специализированное типа futex'ов? Или ещё чего такое, предложенное вендорами компилятора или оси? Как-то так.

Цитата Олег М @ 21.01.17, 08:43

В смысле, ты имеешь ввиду, почему я не делаю AddRef () {return ++m_cnt != 1;}? Потому что так нельзя. Если счётчик равен нулю, то его нельзя увеличивать, иначе когда несколько потоков будут одновременно вызывать AddRef(), только одному вернётся false, остальным - true и указатель на объект который был удалён.

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

Цитата Олег М @ 22.01.17, 07:13

Итак - зачем мне это нужно?
1. Тренировка мозгов. Когда работаешь с потоками ошибку сделать легко, но найти её можно только путём пристального разглядывания и анализа исходного кода. Для этого нужно всегда быть в форме и помнить о нюансах и правилах. Lock-free алгоритмы - хорошая тренировка.

Тренировка мозгов - это, конечно, хорошо. Но на изначальный вопрос это не отвечает: в каких именно сценариях (реальных сценариях) планируется использовать такие указатели? Увы...

Цитата Олег М @ 22.01.17, 07:13

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

Ну... Как сказать... Повторное использование кода - это, конечно, хорошо. Но... Самый хороший многопоточный код - это такой, который не шарит ресурсы между потоками. Вот вообще. То есть когда потоки друг другу не мешают. Когда же появляются разделяемые ресурсы - доступ к ним приходится синхронизировать, да. И потоко-безопасные классы - это не панацея. Совсем не панацея. Именно поэтому я в n-ый раз прошу сценарий. Какая задача решается? Для каких гвоздей нужен этот микроскоп? Ведь, возможно, для такой задачи существует традиционное, давно зарекомендовавшее себя решение.

Цитата Олег М @ 22.01.17, 07:13

3. Минимизация времени блокировки. Не думаю (вернее, не знаю как), что можно заблокировать shared_ptr на меньшее время, причём независимо от других указателей. Например есть большое количество объектов, которые создаются/удаляются в одних потоках, обрабатываются в других. Для решения этой задачи я пользуюсь следующим методом:
- В обработчике хранится список list<weak_ptr> и мьютекс
- Когда мне надо что-то с ним сделать, я блокирую этот мьютекс, переношу список в локальную переменную при помощи move-конструктора, разблокирую
- Пробегаюсь по локальному списку, если weak_ptr::lock возвращает null, удаляю из локального списка, иначе вызываю какую-нибудь функцию
- Снова блокирую список, делаю list::splice, т.е. возвращаю не удалённые объекты обратно
Все операции копеечные и никаких дедлоков

Вот это похоже на более-менее реальный кейс, похожий на что-то типа паттерна producer/consumer или publisher/subscriber. Причём тут ты написал вполне себе адекватную реализацию с помощью "внешнего" мьютекса, который синхронизирует доступ к разделяемому ресурсу (некоему списку). Осталось только понять - что там с shared_ptr'ами, на которые ты weak_ptr'ы хранишь, происходит. И почему тебе нужна грануляция уровня одного объекта, а не коллекций. И почему ты не можешь работать с копиями shared_ptr'ов.

Цитата Flex Ferrum @ 23.01.17, 22:32

Цикл с compare_exchange - это цикл, для которого компилятор сгенерирует какой-то код. А как долго допустимо его крутить? Может есть что-то специализированное типа futex'ов?

Как и любой другой цикл, пока не выполнится условие выхода. Lock-free алгоритмы как правило сходятся, т.е. не приводят к бесконечным циклам.

Цитата Flex Ferrum @ 23.01.17, 22:32

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

Я не знаю, что ты называешь костылями, сам стараюсь не пользоваться этим термином. Если ты знаешь какую-то другую реализацию такого счётчика, просто покажи и всё. Лично я только с такой и знаком.

Цитата Flex Ferrum @ 23.01.17, 22:32

Самый хороший многопоточный код - это такой, который не шарит ресурсы между потоками. Вот вообще.

Не факт, что хороший. Ты наверное хотел сказать - легче всего писать.

Добавлено 24.01.17, 06:03

Цитата Flex Ferrum @ 23.01.17, 22:32

. Осталось только понять - что там с shared_ptr'ами, на которые ты weak_ptr'ы хранишь, происходит. И почему тебе нужна грануляция уровня одного объекта, а не коллекций. И почему ты не можешь работать с копиями shared_ptr'ов.

Потому что shared_ptr - это тоже блокировки, только блокирует она не доступ к объекту, а время жизни объекта. Здесь тоже желательно соблюдать правило минимизации - объект должен жить ровно столько сколько требуется, не больше, не меньше.
Когда ты хранишь где-то список shared_ptr, это значит, что ты должен управлять временем жизни объектов. Для этого тебе придётся написать функцию для удаления объектов из этого списка и гарантировать, что она вызывается везде, где это нужно (причём в деструкторе объекта не получится) и хранить какую-то ссылку на этот список. Ещё и придётся отказаться от списка и использовать map. Всё это, скорее всего, придётся реализовывать каждый раз заново и потом поддерживать, до кучи ещё с дедлоками воевать.
Всё это можно свести к вызовам weak_ptr::lock и shared_ptr::reset. Конечно есть свои нюансы и накладные расходы, но в целом код упрощается в разы и работает надёжнее.

Добавлено 24.01.17, 06:19
Пример такой:
Есть сервер, который генерирует события, и есть обработчики этих событий. Время жизни, создание/удаление, обработчика никак не зависит от этого сервера, и единственное что ему, обработчику, нужно - зарегистрировать себя на сервере, всё. Даже ссылку на сервер хранить не обязательно.
С помощью shared_ptr/weak_ptr эта задача решается легко и красиво. Нет никаких лишних зависимостей, никаких лишних блокировок, лишнего кода и проблем.

Сообщение отредактировано: Олег М - 24.01.17, 06:20

Ага. Значит таки имеет место реализация паттерна pub-sub. Ну, тогда у меня две новости: одна плохая, другая - тоже плохая. Первая: этот паттерн прекрасно реализуется на стандартных смарт-указателях. Вторая: твои экзерсизы с потокобезопасными указателями не избавляют код от одной довольно неприятной гонки (ну, если я правильно понимаю, как там у тебя всё устроено).
Как этот паттерн обычно реализуются у меня. Есть список (вектор, лист, мама - не суть) shared_ptr-ов на дескрипторы подписчиков. В каждом дпскрипторе содержится: указатель (голый) на подписчика, мьютекс и признак того, что подписчик вообще жив. Ну и ещё один мьютекс на весь список. Дальше, реализуются следующие сценарии:
1. Подписка. Создаётся (через make_shared) дескриптор, заполняются поля и под глобальным мьютексом кладется в таблицу.
2. Оповещение подписчиков. Под глобальным мьютексом снимается копия списка. Дальше - обход. Для каждого элемента уже под его лочкой проверяется - жив ли он (в смысле, не случилась ли отписка) и, если жив, под этой же лочкой делается вызов.
3. Отписка. Под глобальной лочкой в списке ищется нужный дескриптор, извлекается в локальную переменную. Не под глобальной, но под локальной лочкой, ей сбрасывается признак "живости" и указатель на клиента, потом делается выход.

Почему это именно так, и почему это работает:
1. Подписчикам (при регистрации) нет необходимости передавать смарт-указатели на себе. Они подписываются и всё. Как хотят - так и подписываются.
2. Указатель на дескриптор клиента умирает тогда, когда умирает последняя копия указателя на него. Тут shared_ptr-используются по прямому назначению.
3. (важно!) обеспечивается синхронизация между отпиской клиента и его нотификацией. Если этой синхронизации не будет, то клиент (каждый!) должен будет работать из расчёта на то, что отписка ничего не гарантирует. И после отписки ему всё равно могут придти вызовы. Клиентов обычно очень расстраивает этот факт.
4. Такая реализация заточена на то, что подписка/отписка - гораздо более редкие операции, чем нотификация. Поэтому лочки мьютексами почти не влияют на производительность.

Не знаю, насколько п.4 для тебя верен, но п.3 твой код точно не реализует за счёт этой вот предварительной оптимизации с атомиками и lock-free.

Ну да, схема тут, незамысловатая. Я, кстати, тоже обычно не передаю указатель на себя. Передаю ссылку на функцию и параметры, если нужно.
Я примерно так и делал, пока не обратил внимания, что такой дескриптор, собственно, дублирует функционал shared_ptr/weak_ptr. Только +мютекс, +флаг и +shared_ptr. Решил, что это как-то накладно.

Цитата Flex Ferrum @ 24.01.17, 07:27

3. (важно!) обеспечивается синхронизация между отпиской клиента и его нотификацией. Если этой синхронизации не будет, то клиент (каждый!) должен будет работать из расчёта на то, что отписка ничего не гарантирует. И после отписки ему всё равно могут придти вызовы. Клиентов обычно очень расстраивает этот факт.

Согласен, в моём случае это действительно проблема. Приходится о ней помнить.
В твоём случае вызов делается под блокировкой и под той же блокировкой происходит отписка. Здесь возможен дедлок, об этом тоже надо постоянно помнить.

Цитата Олег М @ 24.01.17, 08:30

Здесь возможен дедлок, об этом тоже надо постоянно помнить.

Который решается рекурсивным мьютексом. Ведь под него захотят войти в том же потоке, в котором делается вызов.

Добавлено 24.01.17, 08:39

Цитата Олег М @ 24.01.17, 08:30

Я примерно так и делал, пока не обратил внимания, что такой дескриптор, собственно, дублирует функционал shared_ptr/weak_ptr.

Нет. Не дублирует. Хотя бы потому, что это явно прописанная бизнес-логика.
К слову, для массива shared_ptr'ов, в котором хранятся подписчики, можно замутить CoW. То есть в процессе нотификации "шуршать" по оригинальному массиву, а когда приходит параллельный запрос на подписку/отписку - делать копию.

Цитата Flex Ferrum @ 24.01.17, 08:33

Который решается рекурсивным мьютексом. Ведь под него захотят войти в том же потоке, в котором делается вызов.

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

Цитата Олег М @ 24.01.17, 08:44

Цитата Flex Ferrum @ 24.01.17, 08:33

Который решается рекурсивным мьютексом. Ведь под него захотят войти в том же потоке, в котором делается вызов.

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

Дедлок возможен только в том случае, если в процессе вызова нотификатора будет вызвана отписка. То есть в самом вызове. А значит, в том же потоке. Если отписка делается из другого потока, то дедлока не будет

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

Цитата Олег М @ 24.01.17, 09:07

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

Нет. Не возникнет. Поток, в котором делается отписка, честно дождётся, пока завершится вызов. С точки зрения самого "драйвера" подписок-отписок - всё честно. Всегда залочен либо глобальный мьютекс, который держит список (только на период копирования списка), либо один из охраняющих элементы мьютексов. Никаких перекрёстных лочек, двойных лочек и прочее. Если клиент в своём коде не напортачит - всё будет в норме.

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

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

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

Цитата Олег М @ 24.01.17, 10:09

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

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

К слову, для этого и нужна гарантия, что после отписки нотификации не придут.

Сообщение отредактировано: Flex Ferrum - 24.01.17, 10:15

Ну, метод отписки сам лочит много чего. И, я так понял, в деструкторе он должен просто сбросить сбросить флажок, иначе при удалении каждого подписчика нужно будет пробегаться по всему списку.

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

В моём случае подписчик не удалится до тех пор, пока сервер подписок вызывает его метод. Проблема возникнет, если подписчик держит ссылку на своего родителя (который, в свою очередь считает, что он всех отписал и можно умереть) и обращается к ней в это время. Тоже плохо, но дедлока не вызовет. В худшем случае - segmentation fault. Что из них хуже - на любителя.

Цитата Олег М @ 24.01.17, 10:40

Ну, метод отписки сам лочит много чего. И, я так понял, в деструкторе он должен просто сбросить сбросить флажок, иначе при удалении каждого подписчика нужно будет пробегаться по всему списку.

Хм... Тут проще на "псевдокоде":

class SubscribtionManager
{
public:
    void Subscribe(ISubscriber* s)
    {
          auto info = std::make_shared<SubscriberInfo>();
          info.subscriber = s;
 
 
           std::unique_lock<std::mutex> l(m_subscribersGuard);
           m_subscribers.push_back(info);
    }
 
    void Unsubscribe(ISubscriber* s)
    {
          SubscriberInfoPtr ptr;
 
          {
               std::unique_lock<std::mutex> l(m_subscribersGuard);
               auto p = std::find_if(m_subscribers.begin(), m_subscribers.end(), [s](auto subPtr) {return subPtr->subscriber == s;});
               if (p == m_subscribers.end())
                    return;
               ptr = *p;
               m_subscribers.erase(p);
          }
          if (!ptr)
               return;
 
          std::unique_lock<std::mutex> l(ptr->guard);
          ptr->subscriber = nullptr;
    };
 
    void Notify(...)
    {
         std::list<SubscriberInfoPtr> subs;
         {
              std::unique_lock<std::mutex> l(m_subscribersGuard);
              subs = m_subscribers;
         }
 
         for (auto ptr : subs)
         {
               if (!ptr)
                     continue;
 
               std::unique_lock<std::mutex> l(ptr->guard);
               if (!ptr->subscriber)
                      continue;
               ptr->subscriber->Notify();
         }
    }
 
private:
    struct SubscriberInfo
    {
         recursive_mutex guard;
         ISubscriber* subscriber;
    };
 
    using SubscriberInfoPtr = std::shared_ptr<SubscriberInfo>;
 
    std::list<SubscriberInfoPtr> m_subscribers;
    std::mutex m_subscribersGuard;
};

Вот как-то так. Вся синхронизация - на уровне менеджера. А он то уж за своими мьютексами уследить может.

Добавлено 24.01.17, 10:56

Цитата Олег М @ 24.01.17, 10:40

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

С точки зрения языка - это UB. Как только вызывается деструктор - время жизни объекта считается законченным, и вызов методов этого объекта (извне) - это UB.

Метод Unsubscribe у тебя получается очень тяжёлым, почему ты считаешь что эти вызовы должны быть редкими.
Список m_subscribers лучше чистить в Notify - ты там всё равно пробегаешься по списку и заодно удалял бы. Вот как-то так:

class SubscribtionManager
{
public:
    struct SubscriberInfo
    {
        void Unsubscribe() noexcept
        {
            LOCK(guard);
            subscriber = nullptr;
        }
    
        recursive_mutex guard;
        ISubscriber* subscriber;
    };
 
    using SubscriberInfoPtr = std::shared_ptr<SubscriberInfo>;
 
    SubscriberInfoPtr Subscribe(ISubscriber* s)
    {
        auto info = std::make_shared<SubscriberInfo>();
        info.subscriber = s;
 
        std::unique_lock<std::mutex> l(m_subscribersGuard);
        m_subscribers.push_back(info);
        return std::move(info);
    }
 
 
    void Notify(...)
    {
         std::list<SubscriberInfoPtr> subs;
         {
              std::unique_lock<std::mutex> l(m_subscribersGuard);
              for (auto it = m_subscribers.begin(), end = m_subscribers.end(); it != end;)
                    if (it->subscriber)
                        susbs.emplace_back(*it);
                    else    
                        it = m_subscribers.erase(it);
         }
 
         for (auto ptr : subs)
         {
               if (!ptr)
                     continue;
 
               std::unique_lock<std::mutex> l(ptr->guard);
               if (!ptr->subscriber)
                      continue;
               ptr->subscriber->Notify();
         }
    }
 
private:
 
    std::mutex m_subscribersGuard;
    std::list<SubscriberInfoPtr> m_subscribers;
};

Цитата Олег М @ 24.01.17, 11:19

Метод Unsubscribe у тебя получается очень тяжёлым, почему ты считаешь что эти вызовы должны быть редкими.

Считаю, что будут редкими, потому что я об этом в самом начале написал:

Цитата Flex Ferrum @ 24.01.17, 07:27

4. Такая реализация заточена на то, что подписка/отписка - гораздо более редкие операции, чем нотификация. Поэтому лочки мьютексами почти не влияют на производительность.

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

Цитата Олег М @ 24.01.17, 11:19

Список m_subscribers лучше чистить в Notify - ты там всё равно пробегаешься по списку и заодно удалял бы. Вот как-то так:

Нет. Код нотификации работает с копией списка, и должен с ней работать, чтобы гарантировать, что указатели не подохнут, пока он будет по списку идти. Поэтому чистить в коде нотификатора - нельзя. А вот в Unsubscribe - вполне себе. Потому что нужного подписчика всё равно найти надо, сбросить ему всё, тут же можно и итератор из списка выкинуть. Всё под одной лочкой, которая гарантирует, что список сбоку никто не поменяет.

Цитата Flex Ferrum @ 24.01.17, 11:27

Метод Unsubscribe у тебя получается очень тяжёлым, почему ты считаешь что эти вызовы должны быть редкими.

Считаю, что будут редкими, потому что я об этом в самом начале написал:

Я пропустил слово понятно перед словом почему. Даже подумать страшно, сколько раз нужно пробежаться по этому списку, чтоб удалить всех клиентов. Лично я для таких случаев использую map.

Цитата Flex Ferrum @ 24.01.17, 11:27

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

А ну да, я забыл, что ты не хранишь std::shared_ptr<SubscriberInfo> в подписчике. Почему нет? Код, как ты видишь, значительно упрощается. Копирование списка занимает не больше времени, чем его очистка.

Цитата Олег М @ 24.01.17, 11:53

Даже подумать страшно, сколько раз нужно пробежаться по этому списку, чтоб удалить всех клиентов. Лично я для таких случаев использую map.

В случае, если операции подписки/отписки редки - в этом нет ничего страшного. Собственно, можно даже вектор или деку использовать. Да и клиентов в такой модели обычно не много. Для 10-15-ти (да даже 50-ти) - проблем быть не должно.

Цитата Олег М @ 24.01.17, 11:53

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

Потому что я кода не вижу, который упрощается. Кстати, в твоём примере выше ты зачем-то дважды по списку пробегаешься при нотификации. А на спичках хочешь сэкономить.
Как я уже говорил, в случае, если нотификаций в разы (если не на порядки) больше, чем отписок-подписок, для списка подписчиков допустим вариант copy-on-write. То есть только в Subscribe/Unsubscribe делается копия списка, а код нотификации работает с оригиналом. Потом, когда цикл нотификации заканчивается, новый список замещает оригинал.

Цитата Flex Ferrum @ 24.01.17, 12:05

Кстати, в твоём примере выше ты зачем-то дважды по списку пробегаешься при нотификации. А на спичках хочешь сэкономить.

Да ладно, где? Один раз делается копия списка под блокировкой, второй - вызовы. У тебя вроде точно также, тут я ничего не менял.

Цитата Flex Ferrum @ 24.01.17, 12:05

Да и клиентов в такой модели обычно не много. Для 10-15-ти (да даже 50-ти) - проблем быть не должно.

Я обычно рассчитываю на сотни и тысячи. Это для коннектов. Для других объектов - на десятки и сотни тысяч (про миллионы врать не буду). Да и какая разница сколько - сегодня 50, завтра 500, послезавтра 5000. Зачем реализовывать алгоритм плохо, если можно сделать хорошо? тогда и не будет проблем

Цитата Flex Ferrum @ 24.01.17, 12:05

Потому что я кода не вижу, который упрощается

Как минимум, на одну тяжёлую функцию меньше. И на одну тяжёлую операцию быстрее. При сохранении всех прочих преимуществ.

Олег М, ну, как сказать. В моих задачах количество возможных клиентов столько, сколько я привёл. И я, образно говоря, не вижу смысла покупать бентли только чтобы на дачу ездить. У меня в проекте другие узкие места, которые требуют агрессивной оптимизации.
А расскажите, в каких областях требуется управлять тысячам (или сотнями тысяч) подписчиков в рамках паттерна publisher-subscriber? Мне вот даже интересно стало.

Цитата Flex Ferrum @ 24.01.17, 13:14

А расскажите, в каких областях требуется управлять тысячам (или сотнями тысяч) подписчиков в рамках паттерна publisher-subscriber? Мне вот даже интересно стало.

У меня сейчас ~4000 коннектов по сокетам на сервере, это постоянных, на подписке. Каждый из них в свою очередь подписываются на какие-то данные, изменения данных, в рамках того самого паттерна. Примерно раз в секунду происходит коннект-дисконнект, какие-то коннекты отваливаются, но в основном сидят подолгу. Подписаны они на события из базы данных и котировки. Обновления идут очень часто. По событиям из базы данных вызываются процедуры, вообще асинхронно, но определённая синхронизация есть, иначе никаких потоков не хватит. Ещё выполняются скрипты, JavaScript - google V8, тоже асинхронно.

Область - веб-сервер начитывает данные с моего сервера. Онлайн-брокер.

Цитата Flex Ferrum @ 24.01.17, 12:05

Кстати, в твоём примере выше ты зачем-то дважды по списку пробегаешься при нотификации.

Кстати, в моём примере, благодаря отказу от чудовищной Unsubscribe, появилась возможность избежать этой двойной пробежки (или копирования, что одно и тоже):
- под блокировкой сделать move;
- пробежаться по списку, вызвать функции или удалить отписавшихся;
- под блокировкой сделать splice.
Тогда можно будет вообще забыть 10 у тебя клиентов или 10000.

Цитата Flex Ferrum @ 24.01.17, 13:14

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

Это не вопрос "агрессивной оптимизации", это скорее вопрос гигиены.

Сообщение отредактировано: Олег М - 24.01.17, 16:36

А каждый подписывается на свои виды котировок и все получают всё?

Каждый на свой перечень, подписка идёт от клиента, из браузера

Я бы предложил тебе в сторону того, как реализован гугловый gRpc. Возможно, подсмотришь чего интересное. А пока такой вопрос: ты уверен, что правильный паттерн выбрал для реализации? И самое узкое место - в работе с shared-указателями?

Цитата Flex Ferrum @ 24.01.17, 16:52

А пока такой вопрос: ты уверен, что правильный паттерн выбрал для реализации? И самое узкое место - в работе с shared-указателями?

Ну, реализация паттерна, как ты заметил, примерно такая же, как у тебя. Кроме той блокировки, отличий, собственно, не найдёшь. У тебя хорошая, очень грамотная реализация (кроме Unsubscribe!!!).

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

Добавлено 24.01.17, 17:05

Цитата Flex Ferrum @ 24.01.17, 16:52

Wizard

Профиль · PM
Поощрения: 25 Dgm
Рейтинг (т): [ 495 ]
Я бы предложил тебе в сторону того, как реализован гугловый gRpc. Возможно, подсмотришь чего интересное.

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

Сообщение отредактировано: Олег М - 24.01.17, 17:05

Цитата Олег М @ 24.01.17, 16:56

Цитата Flex Ferrum @ 24.01.17, 16:52

А пока такой вопрос: ты уверен, что правильный паттерн выбрал для реализации? И самое узкое место - в работе с shared-указателями?

Ну, паттерн, как ты заметил, примерно такой же, как у тебя. Кроме той блокировки, отличий.

Так вот я и не уверен, что pub-sub тут подходит. Тут наклевывается реактор с producer-consumer или авторами (тут недалеко есть темка с SObjectizer). И асинхронной рассылкой данных.
Кстати, смысла так упарываться со сморт-поинтерами, атомиками и прочим особого не вижу. Освобождение ресурсов, связанных с конкретным коннектом, занимает дохрена времени само по себе. Как, впрочем, и отправка данных в сеть.

Я очень слабо разбираюсь в паттернах, названия мне ничего не говорят

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

С указателями не упарываешься - написал один раз и забыл навсегда

Добавлено 24.01.17, 17:35

Цитата Flex Ferrum @ 24.01.17, 17:09

Тут наклевывается реактор с producer-consumer или авторами (тут недалеко есть темка с SObjectizer).

Где?

Тем не менее. Я так и не уверен, что мой код работает. Кто–нибудь может мне объяснить помогло добавление volatile к std::atomic или нет? Если помогло, то каким образом?

Цитата Олег М @ 24.01.17, 16:33

Это не вопрос "агрессивной оптимизации", это скорее вопрос гигиены.

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

Ссылки:
GRPC
SObjectizer 5.3.0

Цитата Олег М @ 24.01.17, 17:20

Я очень слабо разбираюсь в паттернах, названия мне ничего не говорят
Собственно, сервер подписок работает примерно по той же схеме - копия списка под блокировкой, обработка и т.д. Только обработка заключается в том, что
- для каждого подписчика запускается поток из пула
- он получает список изменённых данных, либо сообщения
- вызывает функцию подписчика

Давай разберёмся. У тебя некий сервер, работающий с браузерами по протоколу HTTP/HTTPS 1.1 (минимум). Браузеры коннектятся к серверу, формируют запрос на котировки, после чего сервер по установленному соединению шлёт запрошенные котировки, пока клиент не отвалится. Таких клиентов - штук 4000 минимум. Чтобы отправить котировки сервер должен обратиться к БД, выбрать нужные, определить клиентов, которым нужны те или иные тикеты, после чего сформировать ответы и послать. В виде, видимо, JSON.
Вопрос номер раз: ты уверен, что игрища с "коллекциями подписчиков" - это наиболее горячее место, требующее оптимизации? А что сказал какой-нибудь Intel VTune? Он вообще что-нибудь сказал?
Вопрос номер два: почему, собственно, тебя беспокоит скорость подписки/отписки клиентов в "менеджере подписок", если это заведомо тяжёлые операции? В первом случае - получить запрос (в виде текста, замечу), распарсить, что-нибудь куда-нибудь положить, модифицировать список получаемых из БД тикетов. Отписка - освободить все ресурсы, память, модифицировать запрос к БД и т. п.
Вопрос номер три: верно ли утверждение, что данные о тикетах отправляются сразу в сокет и в текстовом виде? Если да, то тебя на фоне этой операции действительно беспокоит, насколько долго будут лочится списки подписчиков? Уточню: надо сформировать HTTP-ответ (в текстовом виде), зашифровать его (если работа по SSL), отправить в сокет (пусть и асинхронно), отхендлить ошибки типа отвалившихся клиентов, внезапно упавшей сети и прочие прелести.
Кроме того, 4000 клиентов - это не то, чтобы много, на самом деле, для веб-сервиса.
Вопрос номе четыре: (главный, возвращаясь, так сказать, к началу комментария) а какова, собственно, цель оптимизации? Написать for fun потокобезопасный SmartPointer - это, конечно, весело. Но при работе с продакшен-кодом любая активность должна иметь некую цель, завязанную на бизнес-задачи. В данном случае какую проблему ты решаешь, пытаясь оптимизировать одни этот конкретный участок? И тот ли участок ты оптимизируешь?

Цитата Flex Ferrum @ 24.01.17, 22:40

Давай разберёмся. У тебя некий сервер, работающий с браузерами по протоколу HTTP/HTTPS 1.1 (минимум). Браузеры коннектятся к серверу, формируют запрос на котировки, после чего сервер по установленному соединению шлёт запрошенные котировки, пока клиент не отвалится. Таких клиентов - штук 4000 минимум. Чтобы отправить котировки сервер должен обратиться к БД, выбрать нужные, определить клиентов, которым нужны те или иные тикеты, после чего сформировать ответы и послать. В виде, видимо, JSON.

Сервер, вернее один из серверов, - по сути просто кэширует данные, которые хранятся в базе данных и отдаёт их либо по rpc-запросу, либо по подписке клиенту. Данные в кэше обновляются асинхронно, по сигналам из базы. Ещё, по запросам выполняются ява-скрипты, которые получают данные из памяти сервера, делают какие-то вычисления и возвращают результат.
Он не работает с браузеры, запросы идут от веб-серверов, по сокетам, TCP.

Цитата Flex Ferrum @ 24.01.17, 22:40

Вопрос номер раз: ты уверен, что игрища с "коллекциями подписчиков" - это наиболее горячее место, требующее оптимизации? А что сказал какой-нибудь Intel VTune? Он вообще что-нибудь сказал?
Вопрос номер два: почему, собственно, тебя беспокоит скорость подписки/отписки клиентов в "менеджере подписок", если это заведомо тяжёлые операции? В первом случае - получить запрос (в виде текста, замечу), распарсить, что-нибудь куда-нибудь положить, модифицировать список получаемых из БД тикетов. Отписка - освободить все ресурсы, память, модифицировать запрос к БД и т. п.

Ну, во-первых я там ничего не оптимизирую, там и так всё нормально работает. Во-вторых, меня не беспокоит скорость подписки/отписки - куда ещё быстрее?. Это должно беспокоить тебя.

Цитата Flex Ferrum @ 24.01.17, 22:40

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

У меня не лочатся списки подписчиков. Все нотификации выполняются асинхронно и независимо от других. Я не могу допустить, чтоб все клиенты блокировались из-за одного, либо чтоб данные перестали обновляться, из-за того, что какая-то процедура тормозит. Вернее, определённые блокировки есть, но в основном для того чтоб контролировать количество запущенных потоков.

Цитата Flex Ferrum @ 24.01.17, 22:40

Кроме того, 4000 клиентов - это не то, чтобы много, на самом деле, для веб-сервиса.

У меня не веб-сервис. 4000 (тут, признаюсь, немного приврал, такой цифры я не видел. В среднем - 3500) это коннекты по веб-сокетам, постоянные. RPC-коннекты я не считаю, но в среднем где-то один в секунду.

Цитата Flex Ferrum @ 24.01.17, 22:40

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

У меня сервер работает на машине с 4 процессорами и 16Гб памяти. Хранит данные о 50тыс аккаунтов и 15тыс инструментов, постоянно обновляет их и рассылает подписчикам. Как ты думаешь у меня там есть возможность делать что-то не оптимально (хотя, конечно, косячу регулярно)?

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

Цитата Олег М @ 24.01.17, 20:15

Тем не менее. Я так и не уверен, что мой код работает. Кто–нибудь может мне объяснить помогло добавление volatile к std::atomic или нет?

Поиск мелких багов - довольно утомительная (хотя и интересная) работа.
И не всегда простая.
Кто-же выполнит её за тебя ?
---
А что такое "volatile" ?
Это запрет регистровой оптимизации для переменных.
Т.е. если объявлена некая переменная, компилятор из соображений
оптимизации может разместить переменную в регистр.
Часто это бывает с счётчиками циклов.
---
Ничего подробно не знаю про std::atomic.
Но это шаблонный класс.
Вряд ли его экземпляр разместят в регистре.
Задача этого класса - гарантировать атомарность операций.
Для его пользователя не важно, как он устроен внутри.

Сообщение отредактировано: ЫукпШ - 25.01.17, 09:00

Цитата ЫукпШ @ 25.01.17, 08:57

Поиск мелких багов - довольно утомительная (хотя и интересная) работа.
И не всегда простая.
Кто-же выполнит её за тебя ?

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

Насчёт atomic - да, я тоже раньше думал, что ему не требуется volatile. Видимо, это не так. У него и методы идут парами - один с volatile, другой без.

[quote=Flex Ferrum,1485242843,3703624]3. (важно!) обеспечивается синхронизация между отпиской клиента и его нотификацией. Если этой синхронизации не будет, то клиент (каждый!) должен будет работать из расчёта на то, что отписка ничего не гарантирует. И после отписки ему всё равно могут придти вызовы. Клиентов обычно очень расстраивает этот факт.[/quo

Не рассказал, как я решаю эту проблему (вернее, она решалась довольно коряво, стыдно было рассказывать).
У меня тоже есть дескриптор подписчика, причём в единственном экземпляре, noncopyable. Он хранит у себя shared_ptr на подписчика(теперь CSharedPtr), дублирует его интерфейс (тут, наверное, можно было бы и унаследоваться). Но, методе reset() ждёт, чтобы все экземпляры на его указатель были уничтожены

void reset() noexcept
{
    if (!m_sp)
        return;
 
    TS::CWeakPtr<T> sp = m_sp;
    m_sp.reset();
    while (!sp.expired())
        std::this_thread::yield();
}

Цитата Олег М @ 25.01.17, 09:09

Одна ошибка обнаружилась, вроде ты же на неё и указал, думаю, не особо копаясь в коде.

Да я не полагал, что это - ошибка. Это недостаток.
---
Попробуем сначала.
1. Будем писать шаблонный класс SP - группа объектов указывает на какой-то объект в памяти.
SP должен уметь создать группу, уничтожить группу и объект, присоединиться к группе, отсоединиться
от группы. Обеспечить доступ к объекту. Желательно возможность модификации объекта как-нибудь по-проще.
2. Имеется счётчик ссылок.
3. Предполагаем волюнтаристским способом, что создание или уничтожение группы - потокобезопасно (ПБ).
Поскольку оба действия предполагают наличие только одного объекта SP.

Достаточно таких возможностей ?

4. Значит, надо обеспечить ПБ только на изменение (инкремент/декремент) или установку значения счётчика ссылок.
5. Варианты создания объекта группы:
a) с критической секцией.
б) с использованием "Interlocked" функций WINAPI
MSDN
Выбирай, что больше нравится.
Никто никого нигде специально не ждёт. Модифицируем переменную WINAPI функцией.

Вроде всё.
---
А что за клиент - если не секрет ? Игрушка ?

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

Цитата ЫукпШ @ 26.01.17, 12:17

А что за клиент - если не секрет ? Игрушка ?

Интернет-брокер

Добавлено 27.01.17, 06:33

Цитата ЫукпШ @ 26.01.17, 12:17

Попробуем сначала.
1. Будем писать шаблонный класс SP - группа объектов указывает на какой-то объект в памяти.
SP должен уметь создать группу, уничтожить группу и объект, присоединиться к группе, отсоединиться
от группы. Обеспечить доступ к объекту. Желательно возможность модификации объекта как-нибудь по-проще.

Что-то никак могу взять в толк, что именно требуется. Не доходит.
А можешь как-то грубо накидать в виде классов, без потоков, чисто символически. Мне так проще понять будет.

Цитата Олег М @ 27.01.17, 06:28

А можешь как-то грубо накидать в виде классов,

Вот, сделал оба варианта.
В многопоточном приложении проверил оба, но не запредельно подробно.
Прикреплённый файлShPtrs.zip (4,29 Кбайт, скачиваний: 80)

Сообщение отредактировано: ЫукпШ - 27.01.17, 18:46

Цитата ЫукпШ @ 27.01.17, 17:24

Вот, сделал оба варианта.
В многопоточном приложении проверил оба, но не запредельно подробно.

Т.е. насколько я понял:
- У тебя есть указатель и счётчик ссылок, class Status, который при обнулении которого этот указатель кто-то должен будет удалить, в частности ShPtrIntLockedTh.
- ShPtrIntLockedTh хранит у себя указатель на этот class Status и флажок memArray, который определяет, на что указывает Status, но кроме вызова delete нигде больше не используется.
- Безопасно тебе нужно только удалять содержимое Status.

Правильно?

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

Добавлено 28.01.17, 07:35
Вообще, это поведение очень сильно напоминает поведение shared_ptr, и я бы им и воспользовался.

А так все средства и методы, необходимые для решения этой задачи у меня вроде показаны
- class Status {CRefCounter Count; UT *RealPtr;}
- ShPtrIntLockedTh<T>::DisConnectSlave - деструктор CSharedPtr, вернее строчка if (ref.m_p && ref.m_cnt->m_refs.Release()) delete ref.m_p;
И дальше, по аналогии

По поводу memArray - диковатая довольно штука, особенно с учётом того, что этот флажок можно устанавливать независимо от указателя.

Я бы тут сделал примерно так

template <class T>
class ShPtrIntLockedTh
{
public:
    template <typename T2>
    ShPtrIntLockedTh(std::unique_ptr<T2> sp)
    : m_p(sp.get())
    , m_deleter(sp.get_deleter())
    {
        //Status = new Status() !!!Здесь может возникнуть исключение
 
        sp.release();
    }
 
    ~ShPtrIntLockedTh()
    {
        /////if (....)
            m_deleter(m_p);
    }
protected:
    T *m_p;
    std::function<void(T *)> m_deleter;
};

Сообщение отредактировано: Олег М - 28.01.17, 08:22

Цитата Олег М @ 26.01.17, 12:34

Попробуем сначала.
1. Будем писать шаблонный класс SP - группа объектов указывает на какой-то объект в памяти.
SP должен уметь создать группу, уничтожить группу и объект, присоединиться к группе, отсоединиться
от группы. Обеспечить доступ к объекту. Желательно возможность модификации объекта как-нибудь по-проще.
2. Имеется счётчик ссылок.
3. Предполагаем волюнтаристским способом, что создание или уничтожение группы - потокобезопасно (ПБ).
Поскольку оба действия предполагают наличие только одного объекта SP.

Судя по коду, тебе наверное нужно что-то наподобие вот этого

template <typename T>
class CCountedPtr
{
public:
    template <typename T_>
    static std::unique_ptr<CCountedPtr> Create(std::unique_ptr<T_> sp)
    {
        return sp? std::make_unique<CCountedPtr>(sp): nullptr;
    }
 
    static CCountedPtr *Copy(const CCountedPtr *p) noexcept
    {
        TS_LOCK(GetSharedPtrLock());
        return p && p->m_refs.AddRef()? p: nullptr;
    }
 
    void Release() noexcept
    {
        if (m_refs.Release())
        {
            m_deleter(m_p);
            GetSharedPtrLock().Wait();
            delete this;
        }
    }
 
    T *get() const noexcept
    {
        return m_p;
    }
 
protected:
    template <typename T_>
    CCountedPtr(std::unique_ptr<T_> sp)
    : m_p(sp.get())
    , m_deleter(sp.get_deleter())
    {
        sp.release();
    }
 
    ~CCountedPtr()
    {
        if (m_p)
            m_deleter(m_p);
    }
 
    static CSharedPtrLock &GetSharedPtrLock() noexcept
    {
        static CSharedPtrLock _lock;
        return _lock;
    }
 
    T *m_p = nullptr;
    std::function<void(T *)> m_deleter;
    mutable CRefCounter m_refs;
};
 
template <typename T>
class CSinglePtr
{
public:
    template <typename T_>
    CSinglePtr(std::unique_ptr<T_> sp)
    : m_p(CCountedPtr<T>::Create(std::move(sp)).release())
    {
    }
 
    CSinglePtr(const CSinglePtr &src)
    : m_p(CCountedPtr<T>::Copy(src.m_p))
    {
    }
 
    ~CSinglePtr()
    {
        if (m_p)
            m_p->Release();
    }
 
    void reset()
    {
        if (m_p)
        {
            m_p->Release();
            m_p = nullptr;
        }
    }
 
    T *get() const
    {
        return m_p? m_p->get(): nullptr;
    }
protected:
    CCountedPtr<T> *m_p;
};

На intrlocked-функции заменяется достаточно легко - убираешь атомик и подставляешь вместо его методов и операторов соответствующие функции

Сообщение отредактировано: Олег М - 28.01.17, 11:46

Цитата Олег М @ 28.01.17, 07:19

- Безопасно тебе нужно только удалять содержимое Status.

Правильно?

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

Цитата ЫукпШ @ 28.01.17, 13:18

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

Это и имелось ввиду - безопасно определить, что ссылок больше нет и удалить объект, ShPtrIntLockedTh<T>::DisConnectSlave. Так?

Цитата Олег М @ 28.01.17, 13:25

Это и имелось ввиду - безопасно определить, что ссылок больше нет и удалить объект, ShPtrIntLockedTh<T>::DisConnectSlave. Так?

Да. Если декремент сделан потоко-безопасно и в результате 0,
значит работаем с последним объектом SP в группе.
Всё удаляем.

//Status = new Status() !!!Здесь может возникнуть исключение

Исключение может возникнуть везде.
Перехватим его и получим контекст.

Цитата ЫукпШ @ 28.01.17, 13:30

Исключение может возникнуть везде.
Перехватим его и получим контекст.

Нет, не везде. Это единственное место. Смотри, я там в общих чертах набросал реализацию. По моему как-раз то что тебе нужно

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

Сообщение отредактировано: Олег М - 28.01.17, 13:38

Цитата Олег М @ 28.01.17, 08:20

По поводу memArray - диковатая довольно штука, особенно с учётом того, что этот флажок можно устанавливать независимо от указателя.

Если была акция по созданию массива объектов, тогда их правильно удалять
придётся операцией delete []. Это редкий вариант, но всё-же.
Установить флажок вручную - не сложное мероприятие.
---
Мы же контролируем "вручную", что не делаем 2 равзные группы с одним
указателем на данные ?

Добавлено 28.01.17, 13:39

Цитата Олег М @ 28.01.17, 13:37

Нет, не везде.

Везде.
Ты разименовываешь указатель (где-то), в результате сбоя аппаратных
средств значение казателя становится равным 0...

Цитата ЫукпШ @ 28.01.17, 13:38

Если была акция по созданию массива объектов, тогда их правильно удалять
придётся операцией delete []. Это редкий вариант, но всё-же.
Установить флажок вручную - не сложное мероприятие.

Тебе нужно удалять указатель так же, как он был создан (по-возможности, естеcтвенно). Этого можно достичь при помощи unique_ptr, как я тебе показал. Кроме того, так можно будет гарантировать, что память будет корректно освобождена при исключении в new.

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

Сообщение отредактировано: Олег М - 28.01.17, 14:24

Цитата Олег М @ 28.01.17, 13:52

Цитата ЫукпШ @ 28.01.17, 13:38

Если была акция по созданию массива объектов, тогда их правильно удалять
придётся операцией delete []. Это редкий вариант, но всё-же.
Установить флажок вручную - не сложное мероприятие.

Тебе нужно удалять указатель так же, как он был создан (по-возможности, естеcтвенно).

Да. Допустим, я создал его операцией new [].

Цитата ЫукпШ @ 28.01.17, 14:34

Да. Допустим, я создал его операцией new [].

Тогда тебе его нужно и удалять delete[], желательно гарантированно, без возможности изменить. Для этого лучше воспользоваться классом std::unique_ptr<T[]>, сохранить его функцию удаления и т.д.

Цитата Олег М @ 28.01.17, 14:44

Цитата ЫукпШ @ 28.01.17, 14:34

Да. Допустим, я создал его операцией new [].

Тогда тебе его нужно и удалять delete[], желательно гарантированно, без возможности изменить

Это я и делаю - и вообще, о чём разговор ?
Мне именно такой функционал нужен - я его делаю.
Тебе не нужен - не делай.

Цитата ЫукпШ @ 28.01.17, 14:48

Тебе не нужен - не делай.

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

Цитата ЫукпШ @ 28.01.17, 13:30

Исключение может возникнуть везде

И кстати, не потому ли ты упомянул об этих исключениях, что они у тебя регулярно возникают?
Вообще, так быть не должно. Это следствие неправильно написанного кода. Windows здесь генерит исключение, линукс бы тупо падал по segmentation fault. Но плохо одинаково.
Я тебе показал классы, если ты их смотрел, в которых исключений при обращении к указателю не будет.

Сообщение отредактировано: Олег М - 28.01.17, 17:09

Цитата Олег М @ 28.01.17, 17:07

Цитата ЫукпШ @ 28.01.17, 13:30

Исключение может возникнуть везде

И кстати, не потому ли ты упомянул об этих исключениях, что они у тебя регулярно возникают?

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

Сообщение отредактировано: ЫукпШ - 28.01.17, 20:36

Цитата ЫукпШ @ 28.01.17, 18:57

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

Во-первых, ты так и не ответил на вопрос - возникают ли у тебя исключения при обращении содержимому указателя, либо при операции delete? Насколько я понимаю, очень часто.
Во-вторых, ничего странного в этом нет. Мне бы даже и в голову не пришло сказать, что исключения возникают везде. Они возникают только когда вызывается оператор throw. Оператор delete и разыменование указателя этого не делают.

При таких случаях вызываются системные виндовские исключения, которые, если ты их не отлавливаешь в __try/__except, преобразуются компилятором и выглядят как обычные сишные исключения. Если ты не вызываешь такие исключения специально (есть такая методика, см Рихтера), это означает что у тебя где-то ошибка в коде - ты не проверяешь значение указателя на null, обращаешься к удалённому указателю и т.д. В твоём случае это, обращение к указателям, происходит потому что ты их не синхронизируешь в потоках. Т.е. у тебя там есть какие-то попытки, но они явно не работают.

Думаю, у тебя так ещё куча утечек памяти. Как следствие всего этого.

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

Цитата Олег М @ 29.01.17, 08:45

Цитата ЫукпШ @ 28.01.17, 18:57

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

Во-первых, ты так и не ответил на вопрос - возникают ли у тебя исключения при обращении содержимому указателя, либо при операции delete? Насколько я понимаю, очень часто.

Ответ простой - никогда ещё я не получал исключений, когда использовал этот
шаблонный класс в многопоточном приложении.
Хотя если посмотреть на дату, класс написан буквально к этому обсуждению.
Но прежде чем показывать исходник я конечно проверил, как это работает.
---
Что касается перехвата исключений.
Используя __try/__except действительно можно получить диагностику в виде контекста.
Однако, перехватить любое исключение посредством try/catch тоже можно.
Для этого у компилера от Микрософт существует ключ "/eha".
В этом случае тоже будет использован SEH, только диагностики не получишь.
Так что, если речь идёт о критике принятого решения, критиковать
надо само наличие блоков try/catch, поскольку в этом случае исключение будет перехвачено,
а диагностика получена не будет.
---
Скучно разговаривать - откуда ты можешь знать про утечки памяти и исключения у меня ?
Чтобы выяснить наличие утечек в данном случае нет необходимости проводить сложные исследования.
Достаточно получить диагностику конструкторов/деструкторов создаваемых объектов.

Сообщение отредактировано: ЫукпШ - 29.01.17, 12:30

Цитата ЫукпШ @ 29.01.17, 12:11

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

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

Кстати, пытаюсь сейчас переделать свои указатели под Windows, 32бит, vs2010. Там у меня тоже куча древних серверов, которые как раз и выдают подобные exceptions (если б падали, как в линуксе, уже давно бы починил). Не так просто, оказывается.

Цитата Олег М @ 29.01.17, 12:51

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

"Наука начинается с измерения".©
Для программирования это значит, что самый верный способ - потрассировать, проверить, как работает.

Добавлено 29.01.17, 15:38

Цитата Олег М @ 29.01.17, 12:51

Кстати, пытаюсь сейчас переделать свои указатели под Windows, 32бит, vs2010. Там у меня тоже куча древних серверов, которые как раз и выдают подобные exceptions

Это как-же ?
В Линуксе я никак не могу перехватить все исключения.
Там нет SEH.
Падение чего-то предполагает долгое и упорное исследование.
Кроме того, Виндус некоторые ошибки иногда прощает, Линукс - нет.
---
Для Виндуса делаем так:
Выставляем флаг /eha.
Окружаем, значит, все поточные функции блоками __try/__except с диагностикой.
Если кто-то из них "верещит", начинаем работать.
Можно по-ползать по памяти, изучая исполнимый код отладчиком, но это долго и муторно.
Можно тупо ловить исключение блоками try/catch, уже без диагностики, по алгоритму "ловли льва в африке".
Научное исследование превращается в примитивную и чисто техничекую работу.
Не всегда конечно, но почти.

Сообщение отредактировано: ЫукпШ - 29.01.17, 15:41

Цитата ЫукпШ @ 29.01.17, 15:31

Падение чего-то предполагает долгое и упорное исследование.
Кроме того, Виндус некоторые ошибки иногда прощает, Линукс - нет.

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

Если хочешь, я выложу виндовскии код указателей. Но при условии, что ты мне вызовешь там seh прерывание

Цитата Олег М @ 29.01.17, 17:01

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

Т.е. собеседование ты провалил.
Чтобы быть совсем уверенным, я встроил этот класс в своё приложение, которое должно
работать 24 часа, 365 дней в году. Никаких исключений обнаружить не удалось.

Добавлено 29.01.17, 20:29

Цитата Олег М @ 29.01.17, 17:01

В потоках никакие отладчики и никакие флаги тебе не помогут.

Откуда ты знаешь ?
Если возникнет исключение, я получу контекст, имя файла и стоку в исходниках, в которой
производится вызов фильтра. Если я утверждаю это так определённо, то потому, что неоднократно
это делал.
---
Определить место, где произошло исключение можно достаточно легко.
А вот понять "почему" - действительно не всегда просто. Это "да".

Вот я взял, да и поделил на 0.
Вот полученная диагностика:

Цитата

00000017 23:39:42 [2476] FILE=.\biWth.cpp, Line=159
00000018 23:39:42 [2476] EXCEPTION_INT_DIVIDE_BY_ZERO, ExAddr=004132B6
00000019 23:39:42 [2476] CF =0001003F DR0=00000000 DR1=00000000 DR2=00000000
00000020 23:39:42 [2476] DR3=00000000 DR6=00000000 DR7=00000000 EAX=0000000A
00000021 23:39:42 [2476] EBX=00A1CC40 ECX=00000000 EDX=00000000 EBP=00E4FF68
00000022 23:39:42 [2476] EIP=004132B6 EDI=7C91043E ESI=00A1CC40 ESP=00E4FF14
00000023 23:39:42 [2476] EF =00010246 SCS=0000001B SDS=00000023 SFS=0000003B
00000024 23:39:42 [2476] SGS=00000000 SSS=00000023 SES=00000023 ...=00000000

Добавлено 29.01.17, 20:31

Цитата Олег М @ 29.01.17, 17:01

Если хочешь, я выложу виндовскии код указателей. Но при условии, что ты мне вызовешь там seh прерывание

А зачем ?
Сделай как в книге Рихтера. Внутри блока __try/__except подели на 0
или разименуй нулевой указатель.

Сообщение отредактировано: ЫукпШ - 29.01.17, 20:42

Цитата ЫукпШ @ 29.01.17, 20:26

Откуда ты знаешь ?
Если возникнет исключение, я получу контекст, имя файла и стоку в исходниках, в которой
производится вызов фильтра. Если я утверждаю это так определённо, то потому, что неоднократно
это делал.

Ну, ты мне так и не ответил, какие именно методы должны быть потокобезопасными.
Думаю, что вот эти три:
ShPtrIntLockedTh<T>::ShPtrIntLockedTh(const ShPtrIntLockedTh& p)
ShPtrIntLockedTh<T>::DisConnect(void)/ShPtrIntLockedTh<T>::DisConnectSlave(void)
ShPtrIntLockedTh<T>::operator=(const ShPtrIntLockedTh& p)

Рассмотрим сценарий, когда один поток вызывает конструктор копирования, а другой ShPtrIntLockedTh<T>::DisConnect:
1-й: вызывает DisConnect(), потом DisConnectSlave(), который возвращает true
2-й: вызывает ShPtrIntLockedTh(const ShPtrIntLockedTh& p), проверяет p.StatPtr, который != null, делает StatPtr = p.StatPtr;
1-й: делает delete StatPtr
2-й: вызывает StatPtr->IncCount(); Т.е. пытается изменить удалённый объект

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

Цитата Олег М @ 25.01.17, 07:00

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

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

Цитата Олег М @ 25.01.17, 07:00

Сервер, вернее один из серверов, - по сути просто кэширует данные, которые хранятся в базе данных и отдаёт их либо по rpc-запросу, либо по подписке клиенту. Данные в кэше обновляются асинхронно, по сигналам из базы. Ещё, по запросам выполняются ява-скрипты, которые получают данные из памяти сервера, делают какие-то вычисления и возвращают результат.
Он не работает с браузеры, запросы идут от веб-серверов, по сокетам, TCP.

Цитата Олег М @ 25.01.17, 07:00

У меня не лочатся списки подписчиков. Все нотификации выполняются асинхронно и независимо от других. Я не могу допустить, чтоб все клиенты блокировались из-за одного, либо чтоб данные перестали обновляться, из-за того, что какая-то процедура тормозит. Вернее, определённые блокировки есть, но в основном для того чтоб контролировать количество запущенных потоков.

Цитата Олег М @ 25.01.17, 07:00

У меня не веб-сервис. 4000 (тут, признаюсь, немного приврал, такой цифры я не видел. В среднем - 3500) это коннекты по веб-сокетам, постоянные. RPC-коннекты я не считаю, но в среднем где-то один в секунду.

Итак, java-скрипты, RPC, веб-сокеты, асинхронная работа с БД... Асинхронное оповещение через потоки означает пул потоков и очередь на отправку уведомлений в нём. Нет, определённо, шаренный указатель на клиента - это самое узкое место. Ещё раз: профилировщик это подтвердил? Вот возьми конкретно этот форум, где мы с тобой общаемся. Он довольно коряво написан на PHP, хостится на апаче, сервер - не самый мощный (не такой мощный, как твой). Только вчера он отрабатывал по одному запросу в секунду, на каждый запрос делая семнадцать обращений к базе, рассылая почту, форматируя страницы, и тэ дэ. И успевая всё это сделать за 250 миллисекунд. В свои лучшие времена (посмотри статистику) на ещё более худшем железе от отбивал по десятку клиентских запросов в секунду. На PHP. Не оптимизированном. Без всякого C++ и прочего. Без клиентских скриптов (с их минимумом). В общем, твои показатели не то, чтобы впечатляющие. Ты VTune уже скачал, или продолжаешь визуальный анализ кода на предмет возможных узких мест?

Цитата Олег М @ 25.01.17, 07:00

Во-вторых, меня не беспокоит скорость подписки/отписки - куда ещё быстрее?. Это должно беспокоить тебя.

А вот нужно ли быстрее? А если нужно, то в каком конкретно месте?

Цитата Олег М @ 25.01.17, 07:00

У меня сервер работает на машине с 4 процессорами и 16Гб памяти. Хранит данные о 50тыс аккаунтов и 15тыс инструментов, постоянно обновляет их и рассылает подписчикам. Как ты думаешь у меня там есть возможность делать что-то не оптимально (хотя, конечно, косячу регулярно)?

Есть. Возможность есть всегда. А вот где именно оптимизировать - лучше всего подскажет профилировщик.

Но это я всё ёрничаю. На самом деле, не далее, как вчера, обсуждали с коллегой именно такой тип указателя, о котором ты говоришь. Но... Есть нюанс. Атомики при его реализации будут реальными спичками, ибо там (как я и описывал в своих предыдущих комментариях) вызов методов обёрнут лочкой, при смене указателя вызываются callback'и, есть wait-функции доступности, и прочие подобные вещи. Вот так вот неоптимально. Да и вот сел сегодня, думаю, дай напишу этот самый "потокобезопасный" shared_pointer. Принялся интерфейс weak/shared_ptr'а проектировать, и понял, что ну нет смысла именно эту связку делать потокобезопасной. Не для того она предназначена. Там везде одни сплошные копии самой обёртки, и reset, сделанный на одной копии не аффектит другие. И не должен аффетить: http://ideone.com/LzYlYZ То есть, по сути, тебе не аналог связки weak_ptr/shared_ptr нужен, а что-то "немножко" другое. Что-то надёжное, что ты можешь сохранить один раз, использовать из разных потоков, где-то - лоча, а где-то - перезаписывая.

Цитата Flex Ferrum @ 30.01.17, 21:46

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

Нет, не запускал и не планирую. Что он мне может сказать, чего я и так не знаю?
У меня нет проблем с производительностью и временем обработки запроса, которое в среднем ~0мс. Как ты правильно заметил, количество запросов и объёмы данных у меня не те, чтоб мне понадобилось то, что ты называешь "агрессивной оптимизацией". Тем более я слабо представляю, как можно профилировать сервер с кучей потоков и коннектов. Там в результатах нужно будет разбираться дольше.

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

Цитата Flex Ferrum @ 30.01.17, 21:46

Нет, определённо, шаренный указатель на клиента - это самое узкое место. Ещё раз: профилировщик это подтвердил?

Нет, не определённо. Ты свой код через профилировщик прогонял? Он подтвердил, что функция Unregister() у тебя будет самым слабым местом, при увеличении количества подписчиков?

Цитата Flex Ferrum @ 30.01.17, 21:46

На самом деле, не далее, как вчера, обсуждали с коллегой именно такой тип указателя, о котором ты говоришь. Но... Есть нюанс. Атомики при его реализации будут реальными спичками, ибо там (как я и описывал в своих предыдущих комментариях) вызов методов обёрнут лочкой, при смене указателя вызываются callback'и, есть wait-функции доступности, и прочие подобные вещи. Вот так вот неоптимально.

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

Цитата Flex Ferrum @ 30.01.17, 21:46

Да и вот сел сегодня, думаю, дай напишу этот самый "потокобезопасный" shared_pointer. Принялся интерфейс weak/shared_ptr'а проектировать, и понял, что ну нет смысла именно эту связку делать потокобезопасной. Не для того она предназначена.

Задача shared_ptr сводится к тому, чтоб контролировать время жизни объекта, weak_ptr - чтобы быть в курсе жив объект или нет. Было бы здорово, если бы ты попытался написать его.

Кх-гм... Почему-то представляется диалог:
- Доктор, я себя что-то неважно чувствую.
- А, голубчик, у вас волчанка.
- Доктор, с чего вы взяли? Вы ведь даже анализов у меня не взяли!
- А у вас, вон, глаза красные.

Примерно так выглядят разговоры об оптимизации кода без привлечения данных от инструментальных средств. И именно поэтому "предварительную оптимизацию" называют корнем всех зол - ведь можно потратить массу времени увлеченно оптимизируя то, что в этом совершенно не нуждается.
А профилировать сервер просто: запускаешь его под профайлером, даёшь тестовую нагрузку нужного профиля и снимаешь данные. У тебя ведь есть тестовый стенд, и тестовый же генератор нагрузки?
Оптимизация списка подписчиков, о которой я говорил, была сделана по результатам анализа логов и собранных статистических показателей работы системы - размеры очередей, количество входящих запросов, объёмы исходящих данных, креш-дампы, и т. п. А до этого, в предыдущем проекте - по результатам жёсткого профилирования, где всё крутилась в одном процессе, скорости были на порядки выше и данные снимались не только VTune'ом, но и с помощью Windows Performance Toolkit. Оптимизацией под контролем инструментальных средств я занимаюсь лет десять, а то и больше.

На счёт указателя - есть подозрение, что он тебе не очень подойдёт. Там не будет "спичек" (в смысле, мелких оптимизаций, сделанных без видимой причины), зато будет rw-lock. Такая вот жирная пессимизация.

Добавлено 31.01.17, 07:47
И это, в твоей задаче я не вижу именно shared_ptr. Ты не разделяешь владение своим объектом, не отправляешь копии указателя на него в "свободное" плаванье. Тебе нужен что-то в стиле RemoteObjectHolder. Это не столько указатель, сколько сервисный класс, контролирующий доступ к объекту из разных потоков, его операбельность и прочие подобные фишки. Ну, насколько я понял твою задачу. Ты не планируешь создавать копии этого холдера, ты предполагаешь работать только с одной из всех потоков. Так ведь?

Цитата Flex Ferrum @ 31.01.17, 07:41

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

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

Цитата Flex Ferrum @ 31.01.17, 07:41

На счёт указателя - есть подозрение, что он тебе не очень подойдёт. Там не будет "спичек" (в смысле, мелких оптимизаций, сделанных без видимой причины), зато будет rw-lock. Такая вот жирная пессимизация.

Где ты нашёл у меня rw-lock?

Цитата Flex Ferrum @ 31.01.17, 07:41

И это, в твоей задаче я не вижу именно shared_ptr. Ты не разделяешь владение своим объектом, не отправляешь копии указателя на него в "свободное" плаванье. Тебе нужен что-то в стиле RemoteObjectHolder. Это не столько указатель, сколько сервисный класс, контролирующий доступ к объекту из разных потоков, его операбельность и прочие подобные фишки. Ну, насколько я понял твою задачу. Ты не планируешь создавать копии этого холдера, ты предполагаешь работать только с одной из всех потоков. Так ведь?

Не очень понял, но наверное что-то типа того. Что RemoteObjectHolder делает, чего не умеет shared_ptr?

Цитата Олег М @ 31.01.17, 07:58

Где ты нашёл у меня rw-lock?

У тебя его нет. Он у меня будет.

Цитата Олег М @ 31.01.17, 07:58

Не очень понял, но наверное что-то типа того. Что RemoteObjectHolder делает, чего не умеет shared_ptr?

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

Цитата Олег М @ 31.01.17, 07:58

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

Ну, не знаю. Когда при мне занимались оптимизацией (и когда я сам в ней участвовал) задача была вписаться в те или иные требования по производительности и потребляемым при этом ресурсам. И ставилась она не в формулировках "давайте заоптимизируем вот этот кусок кода", а "видишь график распределения времени по функциям? Вот необходимо, чтобы вот эта, вот эта и вот та части занимали не больше 20% от общего времени работы системы". Или: "При 200 активных подключениях длительностью не менее полминуты и десяти новых подключениях в секунду объём потребления процессора не превышал 30%". Или: "При входящем траффике в 1Gb/sec время генерации исходящих событий не должно превышать 10 сек. при 80% потребления CPU". Как-то так. И от этого плясали.

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

Цитата Flex Ferrum @ 31.01.17, 07:41

Кх-гм... Почему-то представляется диалог:
- Доктор, я себя что-то неважно чувствую.

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

Чтобы пользоваться классом, нужно представлять, как он реализован. Я встречал людей, которые кричали, что C++ отстой, а C рулит и приводили пример как быстро работает массив int[100] по сравнению с std::vector.

Добавлено 31.01.17, 08:32

Цитата Flex Ferrum @ 31.01.17, 08:28

У тебя его нет. Он у меня будет

Именно поэтому я там и воспользовался счётчиком.

Добавлено 31.01.17, 08:33

Цитата Flex Ferrum @ 31.01.17, 08:28

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

Я слабо представляю, как operator-> можно использовать в разных потоках

Цитата Олег М @ 31.01.17, 08:31

Вообще, если ты не заметил, то я пришёл сюда не жаловаться на что либо.

Я заметил. Это просто отвлечённый пример-иллюстрация к тому, как можно "эффективно" принимать решения, не выяснив - что именно не так с системой.

Добавлено 31.01.17, 08:35

Цитата Олег М @ 31.01.17, 08:31

Я слабо представляю, как operator-> можно использовать в разных потоках

Можно. И сам объект, к которому он применяется - можно. Более того, ты именно так и делаешь в своём примере с первой страницы.

Цитата Flex Ferrum @ 31.01.17, 08:28

Ну, не знаю. Когда при мне занимались оптимизацией (и когда я сам в ней участвовал) задача была вписаться в те или иные требования по производительности и потребляемым при этом ресурсам. И ставилась она не в формулировках "давайте заоптимизируем вот этот кусок кода", а "видишь график распределения времени по функциям?

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

Цитата Олег М @ 31.01.17, 08:37

Но, думаю, в большинстве случаев следует начинать с анализа кода, тогда и задачи такой не возникнет.

Во всех случаях надо начинать с анализа статистики работы программы. Если она удовлетворяет требованиям - то и дёргаться смысла нет. А вот если не удовлетворяет - то нужно "брать анализы", везти на рентген-МРТ и прочие инструментальные средства диагностики. И только после этого лечить.

Цитата Flex Ferrum @ 31.01.17, 08:34

Можно. И сам объект, к которому он применяется - можно. Более того, ты именно так и делаешь в своём примере с первой страницы.

Нет, я так не делаю. Ни в коем случае. Этот метод можно вызывать только для копии shared_ptr.

Цитата Олег М @ 31.01.17, 08:39

Нет, я так не делаю. Ни в коем случае. Этот метод можно вызывать только для копии shared_ptr.

Посмотри внимательно. Ещё раз. У тебя в глобальном скоупе weak_ptr и shared_ptr. Ты передаёшь их (по ссылке) в рабочие потоки. Потом каждый из этих потоков начинает активно работать с этими самыми глобальными shared_ptr/weak_ptr. Не с собственными копиями, а с глобальными. То есть у тебя основной поток и два рабочих шарят между собой один shared_ptr и один weak_ptr. Ну, так у тебя написано.

Цитата Flex Ferrum @ 31.01.17, 08:44

Посмотри внимательно. Ещё раз. У тебя в глобальном скоупе weak_ptr и shared_ptr. Ты передаёшь их (по ссылке) в рабочие потоки. Потом каждый из этих потоков начинает активно работать с этими самыми глобальными shared_ptr/weak_ptr. Не с собственными копиями, а с глобальными. То есть у тебя основной поток и два рабочих шарят между собой один shared_ptr и один weak_ptr. Ну, так у тебя написано.

Ты посмотри внимательно: во втором потоке я я вызываю weak_ptr::lock, т.е. создаю новый shared_ptr. И потом работаю только с ним. Это и требуется.

        auto sp = sp2.lock();
        if (sp)
            std::cout << (++i) << ", " << *sp << std::endl;

Добавлено 31.01.17, 09:01
В первом потоке я не обращаюсь к содержимому указателей

Цитата Олег М @ 30.01.17, 06:45

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

Рассмотрим сценарий, когда один поток вызывает конструктор копирования, а другой ShPtrIntLockedTh<T>::DisConnect:
1-й: вызывает DisConnect(), потом DisConnectSlave(), который возвращает true
2-й: вызывает ShPtrIntLockedTh(const ShPtrIntLockedTh& p), проверяет p.StatPtr, который != null, делает StatPtr = p.StatPtr;
1-й: делает delete StatPtr
2-й: вызывает StatPtr->IncCount(); Т.е. пытается изменить удалённый объект

Нет, я полагаю, что ты ошибаешься.
Дело в том, что когда

Цитата

"DisConnectSlave(), который возвращает true"

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

Сообщение отредактировано: ЫукпШ - 31.01.17, 10:29

Цитата ЫукпШ @ 31.01.17, 10:25

Нет, я полагаю, что ты ошибаешься.
Дело в том, что когда

Когда между вызовами ShPtrIntLockedTh(const ShPtrIntLockedTh& p) и StatPtr->IncCount() вызовется DisConnect(), для p.

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

Цитата ЫукпШ @ 31.01.17, 10:25

Попытка использования одного и того-же экземпляра SP из разных архитектурных точек приложения
(например из разных потоков) совершенно не верна.

Не использования, а копирования. Которое и должно быть безопасным. И именно из потоков.

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

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

Не поэтому.
Не падает потому, что я "такого" не делаю, поскольку полагаю, что это не правильно.
Создал объект, затем группу - раздал кому требуется, а дальше - владейте.
Можете ещё подсоединяться к группе/отсоединяться от неё.
---
Если ты хочешь обеспечить потоко-безопасность создания/уничтожения группы,
то это уже другая проблема.
Очевидно, такой синхронизатор не может принадлежать самой группе - тут и спорить не о чем.
Но это означает, что придётся сделать общий синхронизатор для всех объектов SP,
в том числе - разных групп и разных типов. Да, это можно, но недостаток такого
объекта очевиден - все группы связаны друг с другом.
Такое я тоже делал, но мне это не очень интересно.
Вот что получилось:

Прикреплённый файлshared_ptr_Th.zip (2,08 Кбайт, скачиваний: 83)
---
Впрочем, можно предложить совсем уже экзотический вариант.
Можно прекратить процесс автоматического создания/уничтожения группы.
Создавать группы как отдельные объекты (где-то в приложении, массивом например), а SP передавать адреса на них.
Тогда синхронизатор может быть внутри группы.
Получится из этой идеи что-нибудь удобное или нет - не знаю.
Такого я не делал.

Сообщение отредактировано: ЫукпШ - 31.01.17, 12:42

Цитата ЫукпШ @ 31.01.17, 11:41

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

Я правильно тебя понял, что те классы которые ты присылал не работают в потоках. Т.е. никакие методы у них не вызываются одновременно?

Добавлено 31.01.17, 16:15

Цитата ЫукпШ @ 31.01.17, 11:41

Такое я тоже делал, но мне это не очень интересно.
Вот что получилось:

Я правильно понимаю, что csShell cssh(&csp); - это блокировка критической секции? Которая объявлена как static MCS csp;?
В каждом методе?

Цитата Олег М @ 31.01.17, 08:58

В первом потоке я не обращаюсь к содержимому указателей

Ну, давай смотреть.
Первый поток:

sp.reset(new std::string("!!!!!!!!!!!!!!!!!!!!!!!!!!!!11111111111111111111111111111111111111"));
sp2 = sp;

Ты модифицируешь глобальный shared_ptr (sp) и перезаписываешь глобальный же weak_ptr (sp2). Второй поток:

auto sp = sp2.lock();

ты пытаешься лочить значение глобального weak_ptr (sp2). Фактически, получаешь data race на доступе к этому самому weak_ptr. И это логично. Ты хочешь сделать то, что хочешь, но стандартные указатели на это не рассчитаны. Поэтому я и говорю:

Цитата Flex Ferrum @ 31.01.17, 07:41

Тебе нужен что-то в стиле RemoteObjectHolder. Это не столько указатель, сколько сервисный класс, контролирующий доступ к объекту из разных потоков, его операбельность и прочие подобные фишки. Ну, насколько я понял твою задачу.

Цитата Flex Ferrum @ 31.01.17, 19:57

Ты хочешь сделать то, что хочешь, но стандартные указатели на это не рассчитаны.

Да, именно это я и хочу сделать. Собственно, и сделал. Мои классы здесь работают.

Насчёт RemoteObjectHolder - т.е. ты предлагаешь, чтобы создание/удаление объекта занимало время, зависящее от количества объектов? В то время, как shared_ptr/weak_ptr вполне справляются с этим за константное время?
Это как минимум. А вообще, думаю там придется писать довольно много кода, вместо простого reset().

Цитата Олег М @ 01.02.17, 06:33

Цитата Flex Ferrum @ 31.01.17, 19:57

Ты хочешь сделать то, что хочешь, но стандартные указатели на это не рассчитаны.

Да, именно это я и хочу сделать. Собственно, и сделал. Мои классы здесь работают.

Насчёт RemoteObjectHolder - т.е. ты предлагаешь, чтобы создание/удаление объекта занимало время, зависящее от количества объектов? В то время, как shared_ptr/weak_ptr вполне справляются с этим за константное время?
Это как минимум. А вообще, думаю там придется писать довольно много кода, вместо простого reset().

Я не совсем понял, что ты имеешь в виду. Смарт-указатели не создают объекты, они контролируют их время жизни. А тебе (плюс к этому) надо контролировать ещё и доступ. А вот время жизни, к слову, не надо. Семантика обычного shared_ptr тут не подойдёт - копии shared_ptr'a, даже изначально указывающие на один объект, независимы (именно поэтому тебе требуется обновлять weak_ptr после reset). Таким образом выходит, что тебе нужна сущность, которая иниициализируется/обновляется в одном месте (строго в одном), используется строго в другом. Ссылки использования тебе при этом считать не то, чтобы требуется - ты их и так знаешь. Можно, конечно, продолжить извращаться с shared_ptr'ами и их аналогами (потокобезопасными), но это как сову на глобус натягивать.

Сообщение отредактировано: Flex Ferrum - 01.02.17, 06:49

Цитата Flex Ferrum @ 01.02.17, 06:48

Смарт-указатели не создают объекты, они контролируют их время жизни. А тебе (плюс к этому) надо контролировать ещё и доступ. А вот время жизни, к слову, не надо. Семантика обычного shared_ptr тут не подойдёт - копии shared_ptr'a, даже изначально указывающие на один объект, независимы (именно поэтому тебе требуется обновлять weak_ptr после reset).

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


SharedPtr не доступ контролирует, а именно время жизни. Собственно, он принимает решение, когда объект нужно удалить.
Использование - это вызов оператора ->.

Добавлено 01.02.17, 07:16

Цитата Олег М @ 01.02.17, 07:02

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

Т.е. ты не смог написать такой класс?

Кстати у майкрософта, в msdn, по поводу shared_ptr написано следующее

Цитата

Thread Safety
Multiple threads can read and write different shared_ptr objects at the same time, even when the objects are copies that share ownership.

Ну, тут они несколько привирают. Потому что тоже не работает, во всяком случае в 2010 студии.

Цитата Олег М @ 01.02.17, 07:02

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

А вот это всё от конкретных сценариев зависит. shared/weak_ptr - это сценарий, когда свет гасит последний уезжающий. Т. е. может быть сколько угодно ссылок на объекты в разных местах программы, когда последняя отваливается - объект удаляется. Удобно. Но не всегда. У тебя, если я всё правильно понимаю, вопросов с тем, когда именно создавать и удалять объект - нет. Создаётся объект тогда, когда совершается коннект к серверу, удаляется - при дисконнекте, ну или вскоре после. То есть создание/удаление неразрывно связано с состоянием определённого ресурса. Поэтому в этом случае возникает совсем другой вопрос: можно ли в данный конкретным момент пользоваться объектом? Именно поэтому ты пытаешься пользовать связку shared_ptr/weak_ptr. Ну, как я понимаю.

Цитата Олег М @ 01.02.17, 07:02

Т.е. ты не смог написать такой класс?

Конкретно такой - нет. И даже причину назвал: его варианты использования не соответствуют требуемым тобою сценариям. А то, что соответствует, не является shared/weak_ptr'ом.

Цитата Flex Ferrum @ 01.02.17, 08:20

. У тебя, если я всё правильно понимаю, вопросов с тем, когда именно создавать и удалять объект - нет. Создаётся объект тогда, когда совершается коннект к серверу, удаляется - при дисконнекте, ну или вскоре после. То есть создание/удаление неразрывно связано с состоянием определённого ресурса.

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

Цитата Flex Ferrum @ 01.02.17, 08:20

Поэтому в этом случае возникает совсем другой вопрос: можно ли в данный конкретным момент пользоваться объектом?

Пользоваться - т.е. обращаться к содержимому указателя? До, но только в том случае, когда ты владеешь shared_ptr.

Цитата Flex Ferrum @ 01.02.17, 08:20

Конкретно такой - нет. И даже причину назвал: его варианты использования не соответствуют требуемым тобою сценариям. А то, что соответствует, не является shared/weak_ptr'ом.

Не соответствует в чём? Интерфейс вроде такой же, поведение тоже. Отличается только реализация.

Цитата Олег М @ 01.02.17, 08:27

Не соответствует в чём?

Не соответствует хотя бы тем, что тебе необходимо из разных потоков работать с одной копией сильного/слабого указателя. Просто вот этой вот самой необходимостью. Это означает, что ты не можешь раздать по потокам копии указателей на некую сущность и "забыть" про неё. Это означает, что несколько потоков контролируемо разделяют доступ к этой сущности, включая время жизни. Повторюсь: иначе всей этой возни бы не было. Попробуй написать хотя бы код своего примера так, чтобы он работал с таким вариантом объявления лямбд:

threads.emplace_back(std::make_unique<std::thread>([sp, sp2, &stop]()
// ...
 
threads.emplace_back(std::make_unique<std::thread>([sp2, &stop]()
// ...

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

Цитата Flex Ferrum @ 01.02.17, 08:38

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

Это вопрос реализации класса, таких требований к нему нет. Видел, что я выше писал по поводу msdn?

Цитата Flex Ferrum @ 01.02.17, 08:38

threads.emplace_back(std::make_unique<std::thread>([sp2, &stop]()
// ...

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

sp2 - это вроде weak_ptr. И что с ним делать во втором потоке, как не вызывать lock()?

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

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

Добавлено 01.02.17, 09:16
С таким же успехом можно сказать - сделай так, чтоб каждый поток использовал свою копию мьютекса

Цитата Олег М @ 01.02.17, 09:15

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

Нет. Они не сделаны для этого. В том то и фишка. Согласно контракту и идеи shared_ptr'а (любой его реализации) reset/изменение одного указателя (его копии) не влияет на другие. А вот когда тебе понадобилось, чтобы влияло, ты и начал городить огород. Но этот огород - уже не shared_ptr.
Представим себе такую ситуацию:

std::string *sp;
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]()
{
    while(!stop)
    {
        sp = new std::string("!!!!!!!!!!!!!!!!!!!!!!!!!!!!11111111111111111111111111111111111111";
        *sp2 = sp;
    }
}));
 
threads.emplace_back(std::make_unique<std::thread>([&sp2, &stop]()
{
    int i = 0;
    while(!stop)
    {
        auto sp = *sp2;
        if (sp)
            std::cout << (++i) << ", " << *sp << std::endl;
 
        std::this_thread::sleep_for(10ms);
    }
}));
 
WaitStop();
stop = true;
 
for (auto &item: threads)
    item->join();

Я убрал из твоего кода смарт-указатели, заменив их на обычные. Этот код корректен? Нет. Ты работаешь с разделяемым ресурсом (sp/sp2) без синхронизации. Ты имеешь здесь data race. unique_ptr/shared_ptr/weak_ptr/XXXPtr - все они имеют семантику указателя. Они ведут себя, как обычный указатель, by design. Соответственно, любая несихронизированная работа с ними из разных потоков - это нарушение контракта и семантики. Ну вот так вот. А тебе нужно именно это - чтобы изменения, сделанные одним потоком, были "подхвачены" другим. Ты хочешь, чтобы твоя обёртка над указателем по контракту позволяла такое использование. Значит, тебе нужно написать именно такую обёртку. Но это уже не будет контракт shared_ptr'а.

Добавлено 01.02.17, 09:27

Цитата Олег М @ 01.02.17, 09:15

С таким же успехом можно сказать - сделай так, чтоб каждый поток использовал свою копию мьютекса

Вот. До тебя начинает доходить. У тебя есть разделяемый ресурс, от которого ты не можешь избавиться. И ты через этот ресурс обмениваешься данными между потоками. Значит, и контракт "обёртки" должен быть на это рассчитан. Не надо натягивать сову на глобус - ей больно.

Задачи которые решают мьютексы и shared_ptr абсолютно симметричные. Разница состоит в том, что один блокирует кусок кода от доступа, второй блокирует объект от удаления. Больше принципиальных отличий я не вижу.

Сообщение отредактировано: Олег М - 01.02.17, 09:33

Цитата Олег М @ 01.02.17, 09:32

Задачи которые решают мьютексы и shared_ptr абсолютно симметричные. Разница состоит в том, что один блокирует кусок кода от, второй блокирует объект от удаления. Больше принципиальных отличий я не вижу.

Это как с тем сусликом. Отличий ты, может, и не видишь. А они есть. Они в том, что mutex - это (по сути и дизайну) разделяемый между потоками ресурс. Ты имеешь в разных потоках одну и ту же копию мьютекса и работаешь именно с этой копией. А shared_ptr разделяет владение неким ресурсом между разными точками в программе. Сколько точек владения (копий shared_ptr'а) - такова величина счётчика. Но при этом (в отличие от мьютекса) точки владения работают независимо друг от друга. Представь, что ты работаешь не с shared_ptr, а с IUnknown и CComPtr. Объект, с интрузивным счётчиком. Каждый новый указатель на него (по контракту) должен счётчик увеличивать, разрушение указателя - уменьшать. Объект умрёт тогда, когда количество ссылок на него - 0. И это произойдёт в любом случае, сколько бы указателей (безссылочных) на него бы не висело. COM-объекты сами следят за временем своей жизни, на желания клиентов что-то там сделать в одном потоке и прочитать в другом им, в сущности, наплевать. Пришло время умирать - умерли. shared_ptr - то же самое, только счётчик экструзивный. Опять не ясно? Разделение владения между копиями указателя != разделение ресурса между потоками. И не может быть равно.

Сообщение отредактировано: Flex Ferrum - 01.02.17, 09:45

Цитата Flex Ferrum @ 01.02.17, 09:40

Это как с тем сусликом. Отличий ты, может, и не видишь. А они есть. Они в том, что mutex - это (по сути и дизайну) разделяемый между потоками ресурс. Ты имеешь в разных потоках одну и ту же копию мьютекса и работаешь именно с этой копией. А shared_ptr разделяет владение неким ресурсом между разными точками в программе

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

Цитата Олег М @ 01.02.17, 09:46

CSharedPtr это тоже разделяемый между потоками ресурс. Почему вдруг нет?

Так а тебе что нужно то на самом деле? Разделяемый между потоками ресурс или разделяемый между точками владения контроль времени жизни? Судя по всему - первое. А ты топишь за второе.

Цитата Flex Ferrum @ 01.02.17, 09:50

Разделяемый между потоками ресурс или разделяемый между точками владения контроль времени жизни?

Вместо "или" поставь "и", тогда будет правильно. Кстати, довольно распространённая ошибка.

Цитата Олег М @ 01.02.17, 09:54

Вместо "или" поставь "и", тогда будет правильно. Кстати, довольно распространённая ошибка.

В таком случае возвращаемся к:

Цитата Flex Ferrum @ 31.01.17, 07:41

Тебе нужен что-то в стиле RemoteObjectHolder. Это не столько указатель, сколько сервисный класс, контролирующий доступ к объекту из разных потоков, его операбельность и прочие подобные фишки.

Потому что гибрид ежа с ужом - это, в целом, не очень хорошая идея. Если ты делаешь точку обмена данными между потоками (а ты делаешь именно её) - то ты должен вносить соответствующие элементы дизайна в интерфейс. Потому что ты делаешь точку обмена данными, у тебя задача такая. Задачу контроля времени жизни ты всё равно решишь. Но, вполне вероятно, совсем не так, как это делает shared_ptr. Хотя бы потому, что контролировать время жизни ресурса ты будешь методами, отличными от счётчика ссылок. И передача владения объектом из потока в поток у тебя может получиться явной, а не "под ковром". В общем, тут пока тот случай, что микроскоп, конечно, достаточно тяжёл, чтобы им гвозди забивать. Но таки лучше взять молоток.

Сообщение отредактировано: Flex Ferrum - 01.02.17, 10:01

Цитата Flex Ferrum @ 01.02.17, 10:01

Если ты делаешь точку обмена данными между потоками (а ты делаешь именно её) - то ты должен вносить соответствующие элементы дизайна в интерфейс. Потому что ты делаешь точку обмена данными, у тебя задача такая. Задачу контроля времени жизни ты всё равно решишь.

Вроде ж решил. CSharedPtr - это не ресурс, это указатель на ресурс. Что за элементы дизайна? Конструктор копирования и weak_ptr::lock это не они?

Цитата Олег М @ 01.02.17, 10:11

Вроде ж решил. CSharedPtr - это не ресурс, это указатель на ресурс. Что за элементы дизайна? Конструктор копирования и weak_ptr::lock это не они?

Ну так у тебя "указатель на ресурс" и является разделяемым ресурсом (такой вот каламбур), точкой обмена данными, посредством которой некий объект попадает из одного потока в другой.

Ну да. Что в этом плохого? Имея ссылку на CSharedPtr, ты можешь в любом потоке сделать копию, которая будет гарантировать, что объект, на который тот указывает, не удалится, либо уже был удалён.

Цитата Олег М @ 01.02.17, 10:20

Ну да. Что в этом плохого?

Плохого в этом именно вот это и есть:

Цитата Олег М @ 01.02.17, 10:20

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

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

Сообщение отредактировано: Flex Ferrum - 01.02.17, 10:28

Я так и не понял - почему нельзя передавать в другой поток shared_ptr тупо по значению?

Цитата OpenGL @ 01.02.17, 11:27

Я так и не понял - почему нельзя передавать в другой поток shared_ptr тупо по значению?

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

Вспомнил ещё одну задачу, которую я решаю при помощи shared_ptr:
Есть объект, достаточно сложный, который постоянно перезаписывается в одном потоке. И есть несколько потоков, которые читают этот объект, причём операция чтения достаточно долгая - форматирование и всё такое. Чтобы чтение не блокировало запись, я в потоке который пишет создаю новый объект и просто подменяю указатель. Вот как то так:

class CWriter
{
    void ProcessWrite()
    {
        auto sp = std::make_unique<Cobject>();
        m_sp = std::move(sp);
    }
    
    CSharedPtr<CObject> m_sp;
};
 
class CReader
{
    void ProcessRead(CWriter &src)
    {
        auto sp = src.m_sp;
        if (sp)
            sp->Format();
    }
};

В общем, возьмись я делать что-то подобное, я бы наколходил что-то типа этого:

#include <iostream>
#include <atomic>
#include <memory>
#include <thread>
#include <mutex>
#include <chrono>
#include <list>
#include <string>
 
using namespace std::chrono_literals;
 
namespace test
{
// Taken from boost examples
class SpinLock
{
public:
  SpinLock() : m_state(Unlocked) {}
 
  void lock()
  {
    while (m_state.exchange(Locked, std::memory_order_acquire) == Locked)
    {        
      /* busy-wait and no yield */
    }
  }
  void unlock()
  {
    m_state.store(Unlocked, std::memory_order_release);
  }
  
private:
  enum LockState
  {
      Locked,
      Unlocked
  };
  
  std::atomic<LockState> m_state;
};
 
template<typename T>
class ObjectHolder
{
public:
    using Holder = std::shared_ptr<T>;
    ObjectHolder() = default;
    explicit ObjectHolder(T* ptr)
        : m_object(ptr)
    {
    }
    ~ObjectHolder() = default;
 
    Holder Lock() const
    {
        std::lock_guard<SpinLock> lg(m_guard);
        return m_object;
    }
    
    void Reset(T* obj = nullptr)
    {
        Holder newHolder(obj);
        {
            std::lock_guard<SpinLock> lg(m_guard);
            m_object.swap(newHolder);
        }
    }
    
    void Reset(Holder newHolder)
    {
        std::lock_guard<SpinLock> lg(m_guard);
        m_object.swap(newHolder);
    }
    
private:
    mutable SpinLock m_guard;
    Holder m_object;
};
} // test_with_mutex
 
int main()
{
    std::cout << "Hello from TestSharedPointer. sizeof(m) = " << std::endl;
    
    volatile bool stop = false;
    std::list<std::unique_ptr<std::thread>> threads;
    test::ObjectHolder<std::string> obj;
    
    
    for (int n = 0; n < 10; ++ n)
    {
        threads.push_back(std::make_unique<std::thread>([n, &obj, &stop]()
        {
            int idx = 0;
            for (; !stop; ++ idx)
            {
                obj.Reset(std::make_shared<std::string>(std::to_string(n) + "_" + std::to_string(idx)));
            }
        }));
    }
    
    threads.push_back(std::make_unique<std::thread>([&obj, &stop]()
    {
        int i = 0;
        while(!stop)
        {
            auto sp = obj.Lock();
            if (sp)
                std::cout << (++i) << ", " << *sp << std::endl;
    
            std::this_thread::sleep_for(10ms);
        }
    }));
    
    std::this_thread::sleep_for(5s);
    stop = true;
    
    for (auto &item: threads)
        item->join();
}

По сути, сервисный класс, просто контролирующий (на спинлоке) доступ к std::shared_ptr'у из разных потоков, и не дающий его одновременно отресеттить, и скопировать. Синхронизация сделана на спинлоке из рассчёта на то, что обмен указателями (или щёлканье ссылками) в shared_ptr'е происходят достаточно быстро. Кроме того, всё это полагается на то, что внутри shared_ptr'а - атомики и барьеры по памяти. В идеале, конечно, тут в частности (и в таких задачах вообще) требуется мьютекс, сколько бы он не занимал. Потому что mutex создаёт барьер по памяти, и после анлока все изменённые данные становятся видимы всем. Для спинлоков и подобных конструкций такое, естественно, не гарантируется.
Почему не weak_ptr. Потому что слабая ссылка - это слабая ссылка. Она не держит объект. При проходу по коллекции (как обсуждалось ранее) вполне можно использовать shared_ptr::use_count для того, чтобы определять дохлые объекты и вычищать их. Ну, если это вообще требуется.

Добавлено 01.02.17, 22:10
Причём, обращу внимание, под лочкой делается только то, что практически атомарно работает с памятью. Тяжёлые операции типа аллокации нового ref_counter'а или удаления объекта делается без лочки. И, опять же, в операциях аллокации достаточно спотыкача на мьютексах. Это просто к вопросу о том, что такого рода оптимизация в месте, где присутствуют аллокации - примерно как мёртвому припарки. По замерам, больше времени уходит на всякие new/delete, чем на что-либо ещё.

Сообщение отредактировано: Flex Ferrum - 01.02.17, 22:16

Цитата Flex Ferrum @ 01.02.17, 21:59

По сути, сервисный класс, просто контролирующий (на спинлоке) доступ к std::shared_ptr'у из разных потоков, и не дающий его одновременно отресеттить, и скопировать.

Т.е. ты просто навешал мьютекс поверх shared_ptr. Теперь покажи, как ты его навешаешь поверх weak_ptr. Ещё объясни, зачем нужен дополнительный спинлок, если shared_ptr сам по себе уже спинлок?

А вот насчёт вот этих штук - std::memory_order_acquire и т.п., можно поподробнее? Это как раз та вещь в которой я ничего не понимаю и так и не смог понять.

Сообщение отредактировано: Олег М - 02.02.17, 06:22

Кстати, а где конструктор копирования для ObjectHolder?

1. Навесил не мьютекс, а спинлок. Это немного разные вещи.
2. А зачем что-то делать с weak_ptr, если в этих сценариях он не нужен? Для чего тебе нужен именно слабый указатель?
3. Спинлоки и атомики - это принципиально разные вещи. Первые реализуются на базе вторых, но вторые не являются первыми. И в shared_ptr нет спинлоков. Там есть атомики.
4. Эти волшебные константы управляют ордерингом операций с памятью в многопоточной среде. Но я сам в них толком не разбираюсь - проще прозу найти подходящую.

Цитата Flex Ferrum @ 02.02.17, 07:53

1. Навесил не мьютекс, а спинлок. Это немного разные вещи.

Чем же они разные, mutual exclusions?

Цитата Flex Ferrum @ 02.02.17, 07:53

Для чего тебе нужен именно слабый указатель?

Как минимум, потому что он есть. И нужно гарантировать, чтоб он тоже корректно отрабатывал. Это ссылка на ресурс, которая знает, удалён ресурс или нет. Очень полезная вещь.

Цитата Flex Ferrum @ 02.02.17, 07:53

3. Спинлоки и атомики - это принципиально разные вещи. Первые реализуются на базе вторых, но вторые не являются первыми. И в shared_ptr нет спинлоков. Там есть атомики.

Ну да, совсем разные. Спинлок - это цикл, атомик - это блокировка. Увеличение счётчика в цикле - это не спинлок? Посмотри как реализован shared_ptr у майкрософта. Т.е. вместо того, чтоб навешивать внешний мьютекс, достаточно сделать, чтоб счётчик увеличивался в цикле. Ну и удаление счётчика синхронизировалось (кстати, это как раз то, что отсутствует у майкрософта).

Цитата Flex Ferrum @ 02.02.17, 07:53

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

Стоило бы поднять этот вопрос. И разъяснить проблему в более доступных терминах, например в терминах мьютексов. Я прочёл массу статей и ничего не понял. Думаю, я не один такой.

Цитата Олег М @ 02.02.17, 08:22

Чем же они разные, mutual exclusions?

Тем, что мьютекс (в традиционном понимании) - это объект ядра. Даже виндовая критическая секция, которая вся из себя user space, всё равно может свалиться в ядро при длительном ожидании. В случае спинлока - это чисто цикл на уровне user space с маленьким футпринтом как по памяти, так и по аффекту ан перформанс. Ну, при адекватном использовании.

Цитата Олег М @ 02.02.17, 08:22

Как минимум, потому что он есть. И нужно гарантировать, чтоб он тоже корректно отрабатывал. Это ссылка на ресурс, которая знает, удалён ресурс или нет. Очень полезная вещь.

Не. Это "как минимум" в данном случае не работает. У тебя есть сценарии, связанные с таким вот управлением ресурсами, где тебе нужен weak_ptr? Т. е. именно слабая ссылка на указатель, отдельно хранящаяся и требующая инициализации? Нет? Ну так о чём тогда речь? Ещё раз: не нужно смешивать контракты и семантику, и не нужно делать вещи, которые можно не делать.

Цитата Олег М @ 02.02.17, 08:22

атомик - это блокировка

Атомик - это не блокировка. Атомик - это атомарная операция с памятью, предоставляющая некоторые гарантии, которые не предоставляют обычные операции. Поэтому увеличение счётчика в цикле - это не спинлок. Это просто увеличение счётчика в цикле таким образом, чтобы другие потоки это видели. Не надо путать тёплое с мягким.

Цитата Олег М @ 02.02.17, 08:22

Стоило бы поднять этот вопрос. И разъяснить проблему в более доступных терминах, например в терминах мьютексов. Я прочёл массу статей и ничего не понял. Думаю, я не один такой.

В терминах мьютексах это не объяснишь. Тут надо понимать, как работает память в многопоточных и многопроцессорных средах.

Цитата Олег М @ 02.02.17, 07:52

Кстати, а где конструктор копирования для ObjectHolder?

Компилятор не маленький, сам напишет.

Цитата Flex Ferrum @ 02.02.17, 08:33

Не. Это "как минимум" в данном случае не работает. У тебя есть сценарии, связанные с таким вот управлением ресурсами, где тебе нужен weak_ptr? Т. е. именно слабая ссылка на указатель, отдельно хранящаяся и требующая инициализации? Нет? Ну так о чём тогда речь? Ещё раз: не нужно смешивать контракты и семантику, и не нужно делать вещи, которые можно не делать.

А зачем, по-твоему, вообще нужен weak_ptr?

Цитата Flex Ferrum @ 02.02.17, 08:33

Компилятор не маленький, сам напишет.

Напишет копирование твоего SpinLock? Я бы ему это не доверил.

Добавлено 02.02.17, 08:48
Кстати, для реализации класса SpinLock, я бы воспользовался std::atomic_flag

Цитата Олег М @ 02.02.17, 08:43

А зачем, по-твоему, вообще нужен weak_ptr?

Он нужен, чтобы держать "слабую" ссылку на указатель и более точно фиксировать точки владения и точки возможного использования объекта. Типичный кейс - для исключения кольцевых "сильных" ссылок, возникающих в иерархических структурах. В тех случаях, которые мы тут обсуждали, weak_ptr не нужен.

Цитата Олег М @ 02.02.17, 08:43

Напишет копирование твоего SpinLock? Я бы ему это не доверил.

Уверен, он прекрасно справится. Rule of Zero рулит. Но тут его, к сожалению, сложно применить.

Добавлено 02.02.17, 08:52

Цитата Олег М @ 02.02.17, 08:43

Кстати, для реализации класса SpinLock, я бы воспользовался std::atomic_flag

Воспользуйся. Я скопипастил реализацию из документации к бусту.

Цитата Flex Ferrum @ 02.02.17, 08:52

Он нужен, чтобы держать "слабую" ссылку на указатель и более точно фиксировать точки владения и точки возможного использования объекта. Типичный кейс - для исключения кольцевых "сильных" ссылок, возникающих в иерархических структурах. В тех случаях, которые мы тут обсуждали, weak_ptr не нужен.

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

Цитата Flex Ferrum @ 02.02.17, 08:52

Уверен, он прекрасно справится. Rule of Zero рулит. Но тут его, к сожалению, сложно применить.

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

Сообщение отредактировано: Олег М - 02.02.17, 09:07

Цитата Олег М @ 02.02.17, 09:06

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

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

Цитата Олег М @ 02.02.17, 09:06

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

Ну да. Такое действительно возможно.

Цитата Олег М @ 02.02.17, 08:22

И разъяснить проблему в более доступных терминах, например в терминах мьютексов.

Ну вот тут про барьеры памяти на русском.

Цитата Flex Ferrum @ 02.02.17, 09:27

Я привык применять weak_ptr в тех местах, где мне нужно ссылаться на shared_ptr, но копия самого shared_ptr не нужна или опасна (по тем или иным причинам)

И как ты в этом случае обеспечиваешь блокировку?

Цитата Олег М @ 02.02.17, 10:55

И как ты в этом случае обеспечиваешь блокировку?

Какую блокировку? Я не совсем тебя понимаю.

weak-ptr::lock.

Цитата Олег М @ 02.02.17, 11:10

weak-ptr::lock.

А зачем мне делать этот lock, если у меня нет weak_ptr, а есть живой shared_ptr, с которого просто снимается копия? Или ты считаешь, что shared_ptr без weak_ptr нельзя использовать?

Если у тебя есть живой shared_ptr, то тебе и слабый указатель не нужен. Однако, если у тебя есть weak_ptr, то тут тебе нужно обеспечить синхронизацию.

Цитата Олег М @ 02.02.17, 11:29

Если у тебя есть живой shared_ptr, то тебе и слабый указатель не нужен. Однако, если у тебя есть weak_ptr, то тут тебе нужно обеспечить синхронизацию.

Возвращаемся к середине. А для чего мне нужен weak_ptr? Кроме того, с моим примером ты можешь сделать так:

std::weak_ptr<std::string> wp(obj.Lock());

и всё будет работать. Гонок не будет до тех пор, пока ты не начнёшь в разных потоках модифицировать один и тот же weak_ptr. А ты, по идее, не начнёшь. У тебя для передачи данных obj есть.

Цитата OpenGL @ 02.02.17, 10:12

Ну вот тут про барьеры памяти на русском.

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

Добавлено 02.02.17, 11:40

Цитата Flex Ferrum @ 02.02.17, 11:32

Кроме того, с моим примером ты можешь сделать так:

Вопрос, что мне потом делать с этим weak_ptr ?

Добавлено 02.02.17, 11:40
Если я его не могу использоать в другом потоке

Цитата Олег М @ 02.02.17, 11:37

Вопрос, что мне потом делать с этим weak_ptr ?

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

Ну, например можно избавиться от функции Unsubscribe.

Я не знаю, что такое функция Unsubscribe. А если подключу свою телепатию и рискну предположить, о чём речь, то... Эмм... Я бы не стал от неё избавляться.

Думаю, ты понял об чем речь. С планшета опять пишу.

Добавлено 02.02.17, 11:55
WeakPtr позволяет за константное время узнать, жив объект, на который он ссылается или нет. По–моему неплохое свойство

Цитата Олег М @ 02.02.17, 11:52

WeakPtr позволяет за константное время узнать, жив объект, на который он ссылается или нет. По–моему неплохое свойство

Есть ещё как минимум два способа узнать это и оба - за константное время. Тут от сценария использования надо плясать, а не от того, что хочется использоватью

Ну, как ты видишь, здесь я обеспечиваю тебе константное время вне зависимости от сценария

Олег М, проверить use_count или вызвать empty у shared_ptr - это константное время. В чём суть проблемы то? Чего лечим?

Мы разве говорим не об weak_ptr?

Цитата Олег М @ 02.02.17, 12:21

Мы разве говорим не об weak_ptr?

Я, честно говоря, уже затрудняюсь сказать - о чём мы говорим. С моей точки зрения беседа сейчас напоминает что-то типа:
- Скажите, а какой микроском лучше мне взять - электронный или оптический?
- А вам для чего?
- Да вот пару гвоздей нужно забить.
- Ну так возьмите молоток!
- Не, мне не нужен молоток. Вы скажите, какой микроскоп лучше?

Цитата OpenGL @ 01.02.17, 11:27

Я так и не понял - почему нельзя передавать в другой поток shared_ptr тупо по значению?

Насколько я понимаю - "тупо" нельзя, и вот почему.
Предположим, что shared_ptr гарантированно потоко-безопасен.
---
Мы пытаемся из другого потока получить от этого экземпляра shared_ptr
указатель на объект, для работы с данными в другом потоке.
Создавая в другом потоке ещё один экземпляр (2-й) shared_ptr.
В момент присоединения к группе нового экземпляра shared_ptr,
процедура из 1-го потока (вдруг) решит уничтожить 1-й экземпляр shared_ptr,
поскольку для неё данные уже не актуальны.
---
Дальше как повезёт.
Если сначала будет получена ссылка на необходимые данные для 2-го потока,
а потом уничтожен 1-й shared_ptr - хорошо.
Но если наоборот - всё плохо.
1-й был единственным в группе, он уничтожит данные, так необходимые 2-му.
И 2-й получит NULL.
В результате получаем вероятностный исход работы программы.

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

Цитата Олег М @ 02.02.17, 13:16

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

Я пока не увидел "целого класса задач". Я увидел две задачи, одна про publisher/subscriber и управления коллекцией подписчиков, другая - что-то вроде producer/consumer с генерацией и обработкой "жирных" объектов. В обоих случаях "общее решение" предполагало скрещивание двух контрактов, которые обычно не скрещивают, и которые, в общем, и не стоит скрещивать - проблему можно решить иначе и проще.

Язык так и чешется написать что–нибудь по поводу скрещивания.
Но, одна из этих задач решается в разы эффективнее, вторая не требует никаких дополнительных средств синхронизации.
И таких задач десятки, если не сотни.

Цитата Олег М @ 02.02.17, 13:37

Но, одна из этих задач решается в разы эффективнее, вторая не требует никаких дополнительных средств синхронизации.

А позволь уточнить: в чём эффективность решения заключается? Ты работаешь с разделяемым ресурсом, но при этом делаешь вид, что не работаешь с разделяемым ресурсом, а просто ссылочками щёлкаешь. Вроде бы выглядит красиво, по факту - скрытые контракты, смешение контрактов и неявность. Что мешает поступить так, как я показал (с "колхозным" объектом и явной синхронизацией) - я так и не понял. Во втором случае... Та же самая ситуация. Ты пытаешься сэкономить на синхронизациях там, где ты явно предполагаешь тяжёлые и длинные операции. Если запустишь под профайлером, то сможешь увидеть, что даже лочка на мьютексе будет незаметна рядом с аллокациями, форматированием и всем прочем. И опять возвращаемся к сакраментальному: чего лечим то? Какую проблему решаем? Или оптимизируем ради оптимизации?

Цитата Flex Ferrum @ 02.02.17, 13:41

Ты работаешь с разделяемым ресурсом, но при этом делаешь вид, что не работаешь с разделяемым ресурсом, а просто ссылочками щёлкаешь. Вроде бы выглядит красиво, по факту - скрытые контракты, смешение контрактов и неявность.

А ты не заметил, что много с какими ресурсами работаешь таким образом? Например вызывая оператор new?

Цитата Олег М @ 02.02.17, 17:35

Цитата Flex Ferrum @ 02.02.17, 13:41

Ты работаешь с разделяемым ресурсом, но при этом делаешь вид, что не работаешь с разделяемым ресурсом, а просто ссылочками щёлкаешь. Вроде бы выглядит красиво, по факту - скрытые контракты, смешение контрактов и неявность.

А ты не заметил, что много с какими ресурсами работаешь таким образом? Например вызывая оператор new?

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

Цитата Flex Ferrum @ 02.02.17, 19:07

Нет. Не заметил. Во избежание выходных с дебаггером

Зря не заметил. Кстати, я и будни с ним не провожу, вообще не припомню когда пользовался.
Отвлеклись от темы

Цитата Олег М @ 02.02.17, 19:55

Зря не заметил. Кстати, я и будни с ним не провожу, вообще не припомню когда пользовался.

Собственно, почему "баба яга против" и в чём она видит проблему такого мегаполезнейшего примитива. Разделяемые ресурсы в многопоточной среде - это потенциальные источники гонок. Даже если код пишется экспертами, обкладывается тестами и всё такое прочее, гонки всё равно возникают. В моей практике ещё не было такого, чтобы без них обошлось в сложной многопоточной программе. Твой примитив типа "решает проблему". Он прячет фактический обмен между потоками под ковёр. Один поток типа что-то записал, другой - что-то подхватил, тра-ля-ля, вуа-ля. Никакого уведомления о том, что содержимое объекта поменялось, он умер или ещё чего-то, никаких синхронизаций. Просто поток работал-работал с одной копией а потом "хоп", и стала другая. Наверное это удобно. Для любителей "написал и забыл". Но именно сокрытие факта передачи объекта между потоками, неочевидность (из кода) этого механизма - это то, что называется error prone. Плюс ко всему нарушение принципа single responsibility. В итоге я не могу признать такой контракт и такой хелперный класс полезным.

Ты о чём вообще, какие гонки, какая неочевидность?

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

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

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

Сообщение отредактировано: Олег М - 03.02.17, 07:24

Цитата Олег М @ 03.02.17, 07:20

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

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

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

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

Добавлено 03.02.17, 08:04
И да. С его использованием weak_ptr не нужен. Этот объект (по сути) выполняет функции этого самого weak pointer'а в рамках обсуждаемых задач.

Цитата Flex Ferrum @ 03.02.17, 08:03

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

Ещё более выше я вроде приводил пример точно такого же класса, только без лишнего спинлока и с контролем удаления. И что?

Цитата Flex Ferrum @ 03.02.17, 08:03

Смарт-указатель лежит внутри объекта и предоставляется по требованию, его счётчики ссылок щёлкаются, когда требуется, всё контролируемо (ну, за исключением создания копии самого объект

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

Цитата Flex Ferrum @ 03.02.17, 08:03

И да. С его использованием weak_ptr не нужен. Этот объект (по сути) выполняет функции этого самого weak pointer'а в рамках обсуждаемых задач.

Не то что бы не нужен. Скорее всего - не рекомендуется использовать.
Этот объект - всего лишь shared_ptr c мьютексом. Который даже скопировать нельзя.

Наткнулся тут на функции типа rcu_read_lock. Думаю заменить ими static CSharedPtrLock. Кто-нибудь может сказать что-то про них?

Цитата Олег М @ 03.02.17, 09:47

Думаю заменить ими static CSharedPtrLock. Кто-нибудь может сказать что-то про них?

Не стоит этого делать. Это функции ядра линукса, используемые для реализации механизмов синхронизации и барьеров по памяти.

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

Кстати, кроме всего вышесказанного, там есть ещё одна, довольно интересная тема, а именно - преобразование типов. Насколько мне известно std::shared_ptr не требует наличия виртуального деструктора для типов, достаточно простой возможности приведения.

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

    void CSharedPtr::reset(CSharedPtr &&sp)
    {
        CSharedPtr sp1(*this);
        sp._swap(*this);
    }

Проявляется ошибка очень редко, как её повторить я не знаю. Описывал её выше - ощущение, что CWeakPtr::lock() возвращает не null для удалённого CSharedPtr.

Добил-таки! Удалось написать код, при котором эти указатели не работают. Теперь всё нормально.
Как я и думал, проблема заключалась в методе CSharedPtr::reset(), который теперь выглядит так

    void reset(CSharedPtr &&sp)
    {
        auto ref = m_ref.load(std::memory_order_acquire);
        if (m_ref.compare_exchange_strong(ref, sp.m_ref.load(std::memory_order_relaxed)))
            sp.m_ref.exchange(ref, std::memory_order_relaxed);
    }

Кроме того, обнаружил неприятный побочный эффект, который заключался в следующем:
Есть указатель CSharedPtr sp. В одном потоке постоянно изменяется его значение, sp.reset(new ....), но он никогда не равен null.
Второй поток копирует этот указатель, CSharedPtr sp2 = sp. Т.е., по идее, sp2 тоже никогда не должен быть null.
Однако, периодически было, что sp2 == nullptr.

Тоже починил.