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

[Flex Ferrum]

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

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

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

[Flex Ferrum]

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

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

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

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

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

[Олег М]

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

[Flex Ferrum]

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

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

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

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

[Олег М]

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

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

[Flex Ferrum]

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

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

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

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

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

[Олег М]

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

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

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

[Flex Ferrum]

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

[Flex Ferrum]

Цитата Олег М @ 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> в подписчике. Почему нет? Код, как ты видишь, значительно упрощается. Копирование списка занимает не больше времени, чем его очистка.

[Flex Ferrum]

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

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

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

[Flex Ferrum]

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