Added test for broker side offline messages. #732
Conversation
|
Now, CI is running but I got Segmentation Fault on my local environment. Here is the log: And here is backtrace (from bottom to top) It seems that the access violation happened at subscription_map.hpp /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../include/c++/10.2.0/bits/hashtable.h │ 1560 // Find the node whose key compares equal to k in the bucket bkt. │
│ 1561 // Return nullptr if no node is found. │
│ 1562 template<typename _Key, typename _Value, │
│ 1563 typename _Alloc, typename _ExtractKey, typename _Equal, │
│ 1564 typename _H1, typename _H2, typename _Hash, typename _RehashPolicy, │
│ 1565 typename _Traits> │
│ 1566 auto │
│ 1567 _Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal, │
│ 1568 _H1, _H2, _Hash, _RehashPolicy, _Traits>:: │
│ 1569 _M_find_before_node(size_type __bkt, const key_type& __k, │
│ 1570 __hash_code __code) const │
│ 1571 -> __node_base* │
│ 1572 { │
│ 1573 __node_base* __prev_p = _M_buckets[__bkt]; │
│ 1574 if (!__prev_p) │
│ 1575 return nullptr; │
│ 1576 │
│ >1577 for (__node_type* __p = static_cast<__node_type*>(__prev_p->_M_nxt);; /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../include/c++/10.2.0/bits/hashtable.h │ 689 __node_type* │
│ 690 _M_find_node(size_type __bkt, const key_type& __key, │
│ 691 __hash_code __c) const │
│ 692 { │
│ >693 __node_base* __before_n = _M_find_before_node(__bkt, __key, __c); /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../include/c++/10.2.0/bits/hashtable.h │ 1442 template<typename _Key, typename _Value, │
│ 1443 typename _Alloc, typename _ExtractKey, typename _Equal, │
│ 1444 typename _H1, typename _H2, typename _Hash, typename _RehashPolicy, │
│ 1445 typename _Traits> │
│ 1446 auto │
│ 1447 _Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal, │
│ 1448 _H1, _H2, _Hash, _RehashPolicy, _Traits>:: │
│ 1449 find(const key_type& __k) │
│ 1450 -> iterator │
│ 1451 { │
│ 1452 __hash_code __code = this->_M_hash_code(__k); │
│ 1453 std::size_t __bkt = _M_bucket_index(__k, __code); │
│ >1454 __node_type* __p = _M_find_node(__bkt, __k, __code); /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../include/c++/10.2.0/bits/unordered_map.h │ 918 iterator │
│ 919 find(const key_type& __x) │
│ >920 { return _M_h.find(__x); } /home/kondo/work/mqtt_cpp/test/subscription_map.hpp │ >181 map_type_iterator get_key(path_entry_key key) { return map.find(key); } /home/kondo/work/mqtt_cpp/test/subscription_map.hpp │ 608 // Remove a value at the specified handle │
│ 609 // returns the number of removed elements │
│ 610 std::size_t erase(handle const &h, Key const& key) { │
│ 611 // Find the handle in the map │
│ >612 auto h_iter = this->get_key(h); /home/kondo/work/mqtt_cpp/test/test_broker.hpp │ 1779 void unsubscribe_all() { │
│ 1780 for (auto const& h : handles) { │
│ >1781 subs_map_.value().get().erase(h, client_id); /home/kondo/work/mqtt_cpp/test/test_broker.hpp │ 1771 void clean() { │
│ 1772 topic_alias_recv = MQTT_NS::nullopt; │
│ 1773 inflight_messages.clear(); │
│ 1774 offline_messages.clear(); │
│ 1775 qos2_publish_processed.clear(); │
│ >1776 unsubscribe_all(); /home/kondo/work/mqtt_cpp/test/test_broker.hpp │ 1733 ~session_state() { │
│ >1734 clean(); |
|
I've fixed it by myself. It was member variable order (lifetime) problem. |
order of destruction was not correct ? subs_map destroyed first, sessions destroyed and trying to unregister from non-existing subs_map ? |
Yes. subs_map_ is destructed earlier than sessions_ on test_broker destruction. |
ah yes, did we not have a discussion before about reference_wrapper vs shared_ptr ? :) or is totally unrelated ? ;) |
Codecov Report
@@ Coverage Diff @@
## master #732 +/- ##
=======================================
Coverage 84.68% 84.68%
=======================================
Files 47 47
Lines 7026 7026
=======================================
Hits 5950 5950
Misses 1076 1076 |
|
Now, v5 tests are added. When CI passed, I will merge the PR.
It difficult to describe but memory violation is better than hiding the design problem using shared_ptr abuse for me. You might have a different design policy. But for mqtt_cpp, please respect my policy. |
|
All CI tests passed. |
No problem. But i would add comment that you are dependent on order of destruction of two data structures. Or possibly enforce the order somehow. |
If I use rust programming language, I guess that enforcing it easily (maybe automatically). But I don't know a good way to do that in C++. One option is compile with address sanitizer. It discover access violation quickly (not perfect). I've already added it on CI. |
|
It is off topic. I tried writing multi_index by rust but it is extremely difficult for me. So I gave up it ;) |
|
I find it interesting to understand why you would choose this pattern and learn from your approach. Maybe I have more theoretical approach than practical :) If you use rust, you really have consider ownership and references much more. Because the borrow checker disallows many common C++ pattern, many are considered not valid. Even though they are practical and are valid in practice. Not sure how many people will use your library if you use Rust, I think it has more value now in C++ :) |
No description provided.