1. C++ / Говнокод #12765

    +13

    1. 01
    2. 02
    3. 03
    4. 04
    5. 05
    6. 06
    7. 07
    8. 08
    9. 09
    10. 10
    11. 11
    12. 12
    13. 13
    14. 14
    15. 15
    16. 16
    17. 17
    18. 18
    19. 19
    20. 20
    21. 21
    22. 22
    class session {
    public:
        session(int id, boost::asio::io_service &io_service) :
            id(id),
            timer(io_service)
        {
            timer.expires_from_now(session_timeout);
            timer.async_wait(boost::bind(&session::on_timeout, this, _1));
        }
    
        void on_timeout(const boost::system::error_code &error) {
            if (error)
                return;
            std::cout << "Session timed out " << id << std::endl;
        }
    
    private:
        int id;
        boost::asio::deadline_timer timer;
    };
    
    std::map<boost::asio::ip::udp::endpoint, boost::shared_pointer<session> > sessions;

    sessions.erase(endpoint) приводит к небольшому насилию над трупом сессии... Ничего конечно не вылетает, и никогда не сломается, но совесть мучает, неприятно пользоваться UB'ом.

    Запостил: bormand, 18 Марта 2013

    Комментарии (45) RSS

    • > boost::shared_pointer
      ?
      Ответить
      • boost::shared_ptr конечно же.
        Ответить
      • Синтетика. Спалился, не проверив в компиляторе.
        Ответить
        • Полусинтетика. Обрезал до минимума, оставляя только идею и проблему, и не проверил в компиляторе.
          Ответить
    • одной из фич asio::deadline_timer является то, что этот таймер не очень пригоден для отсчета таймаутов - он, в соответствии со своим названием, deadline
      т.е. каждый таймер имеет четкую метку во времени, когда надо сработать - и недостаток основной специализации asio::deadline_timer в том, что она построена поверх posix_time, т.е. даты-времени
      при совпадении пары условий (зависит от реализации в конкретной версии буста, но на моей памяти примерно достаточно, чтобы в очереди был другой таймер короче, например) и переводе системных часов назад - например, на 1 сутки, можно ждать не 30 секунд, а 1 сутки и 30 секунд
      аналогично при переводе вперед - можно сломать "хорошие" соединения, посчитав их истекшими

      вряд-ли кого то это сильно коснется, но я не мог это не рассказать, т.к. это когда-то решал)
      Ответить
      • > является то, что этот таймер не очень пригоден для отсчета таймаутов
        Так а в asio других таймеров вроде бы и нет?
        Ответить
        • вечером покажу relative_timer
          Ответить
          • вопщем вот
            http://pastebin.com/mmQ9VWje

            я помню свою радость, когда в буст добавили chrono, хоть и простая, но очень полезная либа
            Ответить
            • показать все, что скрыто
              struct steady_time_traits
              {
                  typedef boost::chrono::high_resolution_clock::time_point    time_type;
              
                  struct castable_duration {
              
                      typedef time_type::duration                 chrono_duration;
                      typedef boost::posix_time::time_duration    posix_duration;
              
                      castable_duration(chrono_duration const & cd = chrono_duration())
                          : chrono_duration_(cd)
                      {}
              
                      castable_duration(posix_duration const & pos)
                          : chrono_duration_(boost::chrono::microseconds(pos.total_microseconds()))
                      {}
              
                      template <class ChronoRep, class ChronoPeriod>
                      castable_duration(boost::chrono::duration<ChronoRep, ChronoPeriod> const & cd)
                          : chrono_duration_(cd)
                      {}
              
                      template <class CastableToPosix>
                      castable_duration(CastableToPosix const & ctp)
                          : chrono_duration_(boost::chrono::microseconds(static_cast<posix_duration>(ctp).total_microseconds()))
                      {}
              
                      operator chrono_duration & ()               { return chrono_duration_; }
                      operator chrono_duration const & () const   { return chrono_duration_; }
              Ответить
              • показать все, что скрыто
                operator posix_duration () const {
                            return boost::posix_time::microseconds(
                                boost::chrono::duration_cast<boost::chrono::microseconds>(chrono_duration_).count());
                        }
                
                    private:
                        time_type::duration     chrono_duration_;
                    };
                
                    typedef castable_duration                                   duration_type;
                
                    /// all below is required by asio
                    static time_type now() {
                        return time_type::clock::now();
                    }
                
                    static time_type add(time_type const & t, duration_type const & d) {
                        return t + static_cast<duration_type::chrono_duration>(d);
                    }
                
                    static duration_type subtract(time_type const & t1, time_type const & t2) {
                        return t1 - t2;
                    }
                
                    static bool less_than(time_type const & t1, time_type const & t2) {
                        return t1 < t2;
                    }
                
                    static boost::posix_time::time_duration to_posix_duration(duration_type const & d) {
                        return static_cast<boost::posix_time::time_duration>(d);
                    }
                };
                
                typedef boost::asio::basic_deadline_timer<steady_time_traits::time_type, steady_time_traits>    relative_timer;
                Ну что за любовь скрывать код за семью ссылками?
                Ответить
                • > Ну что за любовь скрывать код за семью ссылками?
                  Листать же потом неудобно эти портянки. А спойлеров тут нет. Потому и внешний линк.
                  Ответить
                  • > А спойлеров тут нет. Потому и внешний линк.
                    Для говнокода это не проблема. Народ, навались, создай спойлер в комментах с кодом, заминусовав!
                    Ответить
                • Было легче сначало метод to_posix_duration написать, а потом его вызывать из оператора преобразования
                  Ответить
            • > я помню свою радость, когда в буст добавили chrono, хоть и простая, но очень полезная либа
              Я все жду когда в бууст добавят unique_ptr наконец
              Ответить
              • как ты его сделаешь без поддержки компилятором move semantics
                Ответить
                • Уже сделали. Ограничений конечно много, но все равно приятно.
                  http://www.boost.org/doc/libs/1_53_0/doc/html/move.html
                  Ответить
                  • я лучше дождусь компилятор, поддерживающий 11 стандарт, чем буду настолько париться
                    собсно, думаю, в этом году все подтянутся к нужной мне планке
                    Ответить
    • А нельзя в деструкторе сессии отменять коллбэк?
      Ответить
      • он и отменяется - вызывается с ошибкой asio::error::operation_aborted
        не всегда код написан так, что вызов метода on_my_timer у мертвого объекта это переварит (да и это само по себе ub, как было написано автором)
        но обычно, если лень/медленно возиться с shared_from_this(), все на это забивают, главное выйти по ошибке до того, как полезешь в свои члены
        Ответить
        • кстати да, @bormand, раз уж там какой то намек на shared_ptr<session>, и вдруг это session никак иначе не создается, то не вижу причин пренебречь noncopyable, enable_shared_from_this и засунуть таки в бинд shared_from_this()
          Ответить
          • C shared_from_this() у меня другая проблема - в сессии лежит куча подобъектов, которым я отдаю коллбеки и интерфейсы от сессии. И если я буду биндить колбеки с shared_from_this, а интерфейсы буду передавать через shared_ptr, то сессия будет жить вечно пока не вымрут все колбеки и интерфейсы.

            Вот и остается два варианта:
            1) Не заморачиваться с shared_from_this и насильно удалять сессию вместе со всеми ее кишками, забив на UB, как я и показал в топике. Если в колбеках не забывать обрабатывать ошибки - все корректно деинициализируется, но на душе остается неприятный осадок.
            2) Заморочиться с shared_from_this, а для кошерного завершения сессии вызывать cleanup(), который вызовет у подобъектов cleanup() который вызовет... Ну, в общем, куча бойлерплейта отменяющего таймеры да подчищающего shared_ptr'ы.

            Может я упускаю какое-то более изящное решение?
            Ответить
            • > то сессия будет жить вечно
              Вот почему нет weak_from_this? :/
              Ответить
            • > Может я упускаю какое-то более изящное решение?
              C shared_from_this получить shared_ptr, а потом с него получить weak_ptr
              Ответить
              • А boost::bind разве совместим с weak_ptr? Он же всяко не додумается конвертить его в shared_ptr и проверять на NULL при вызове... Да и при вызове интерфейсных методов мне weak_ptr будет только мешаться...
                Ответить
                • А вот это проблема бинда... Хоть свой пиши(
                  Ответить
                  • Написать свой boost::bind уебещся, а вот написать свой аналог std::bind1st, поддерживающий boost::weak_ptr как мне кажется не проблема. Я бы так и сделал наверное
                    Ответить
                    • http://liveworkspace.org/code/1AT0wQ$0
                      Ответить
                      • да можно даже средствами с++03 в лоб, просто не так элегантно будет:
                        boost::weak_ptr<myclass> weak_this(shared_from_this());
                        boost::bind(&myclass::on_weak_member, this, weak_this, ...);
                        
                        void on_weak_member(boost::weak_ptr<myclass> me_weak, ...)
                        {
                           boost::shared_ptr<myclass> me_strong = me_weak.lock();
                           if (!me_strong)
                              return;
                           // use me_strong-> or this-> directly
                        }
                        на самом деле я ни разу еще не сталкивался с тем, чтобы биндить именно вик пойнтер, как я с этим живу?
                        Ответить
                      • #include <iostream>
                        #include <string>
                        #include <boost/bind.hpp>
                        #include <boost/function.hpp>
                        #include <boost/shared_ptr.hpp>
                        #include <boost/weak_ptr.hpp>
                        
                        
                        template <class T, class... A> class weak_member_helper {
                        public:
                            weak_member_helper(void (T::*f)(A...), boost::weak_ptr<T> w) : f(f), w(w) {}
                            void operator()(A... args) {
                                boost::shared_ptr<T> p = w.lock();
                                if (p) {
                                    (p.get()->*f)(args...);
                                } else {
                                    std::cout << "weak object is null!" << std::endl;
                                }
                            }
                        private:
                            void (T::*f)(A ...);
                            boost::weak_ptr<T> w;
                        };
                        
                        template <class T, class...A> boost::function<void (A...)> weak_member(void (T::*f)(A...), boost::weak_ptr<T> w) {
                            return weak_member_helper<T, A...>(f, w);
                        }
                        
                        struct test {
                            test() { std::cout << "test::test()" << std::endl; }
                            ~test() { std::cout << "test::~test()" << std::endl; }
                            void run(int id, std::string s, int &res) {
                                std::cout << "test::run(" << id << ",\"" << s << "\")" << std::endl;
                                res = 5;
                            }
                        };
                        
                        void without_reset() {
                            boost::shared_ptr<test> s(new test());
                            boost::weak_ptr<test> p(s);
                            boost::function<void (int &r)> f = boost::bind(weak_member(&test::run, p), 100500, "test", _1);
                            int x = 2;
                            f(x);
                            std::cout << x << std::endl;
                        }
                        
                        void with_reset() {
                            boost::shared_ptr<test> s(new test());
                            boost::weak_ptr<test> p(s);
                            boost::function<void (int &r)> f = boost::bind(weak_member(&test::run, p), 100500, "test", _1);
                            s.reset();
                            int x = 2;
                            f(x);
                            std::cout << x << std::endl;
                        }
                        
                        int main() {
                            std::cout << "without reset:" << std::endl;
                            without_reset();
                            std::cout << "with reset:" << std::endl;
                            with_reset();
                            return 0;
                        }

                        Я вот тоже подумывал написать нечто подобное, только я собираюсь в рамках С++03 и boost.
                        Ответить
            • для двустороннего хранения шаред ссылок, и когда надо специально, вместо шаред связь может быть weak

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

                > weak
                Вот насчет weak_ptr надо подумать, может не так он и мешается, раз уже двое мне его советуют ;)
                Ответить
                • даже если ты сделал ровно 1 поток для io_service::run, тем более, если всего лишь strand, ты всё равно можешь словить описанный мной баг - просто из природы io_service - это очередь,
                  в эту очередь встанет коллбек на on_timer(error_code()), и пусть ты при этом "однопоточно" деструктишь сессию, коллбек тебе придет именно так, как он был запощен

                  спасет только io_service.stop, что не есть решение
                  Ответить
                  • > ты всё равно можешь словить описанный мной баг
                    OMG... дочитал документацию по cancel() до конца, и понял в чем ужас ситуации: These handlers can no longer be cancelled, and therefore are passed an error code that indicates the successful completion of the wait operation.

                    Т.е. я так понимаю адекватно отменить таймер невозможно?
                    Ответить
                  • Ну правда в случае 2 я смогу перед отменой таймеров загнать сессию в состояние closed, в котором она будет игнорировать все колбеки. А когда все колбеки пробегут, shared_ptr уничтожит сессию. Походу остается только такой вариант.
                    Ответить
    • Да, при наличии сборщика мусора асинхронное программирование таки гораздо удобнее.
      Ответить
      • Я того же мнения. Cборщик мусора есть. Только его никак не пропихнут в буст. Уже год ждет ревьювера кода. boost::block_ptr
        Ответить
      • Мне reference count'а вполне хватает с головой.
        Ответить
        • >reference count
          It unusable at asynchronous programming. Inconvenient as ass.
          Ответить
          • Why not? Can you give some examples when it's inconvenient as fuck? Maybe you're just doing it wrong? With a couple of simple wrappers it can be pretty handy. Sure if you're manually doing the same thing over and over again, it can get very tiresome.
            Ответить
    • >std::map<boost::asio::ip::udp::endpoint , boost::shared_pointer<session> > sessions;
      boost::ptr_map<boost::asio::ip::udp::endpoint,  session> sessions;
      Ответить
      • О, спасибо. Не знал про такой класс. Для эксклюзивного владения вполне подойдет.
        Ответить
    • >неприятно пользоваться UB'ом.
      UB - КРЕСТОСТАНДАРТЕН
      Ответить
    • Я твой UB шатал.
      Ответить

    Добавить комментарий