Receiver node from "develop" branch dies due to accessing nullptr

[marco-tranzatto]

Hi @xqms

We've got an issue with the receiver node dying because of segfault (exit code -11) when using the "develop" branch.

@tomlankhorst and I tried to debug this issue and could get a backtrace using gdb (see end of the message). We were wondering if you could provide us with your feedback about this issue. This is our setup:

• a base station runs the receiver node
• an agent runs the sender node
• the agent sends both tcp (sporadic messages) and udp (images and tf) topics

When the agent goes out of connection range and comes back after some minutes, we often get the receiver node dying on the base station. We suspect that Depacketizer::addPacket is called asynchronously on the same object (for example when 2 UDP packets are received) and so addPacket is called twice. But if it's called async, then it will mess up because the value of the iterator will be changing.

Do you have any hint or suggest on how we could fix this issue?

Best,
Marco

Backtrace:

#0  0x00007ffff7d6ce72 in nimbro_topic_transport::Depacketizer::handleMessagePacket (this=0x7fffffffbfb0, it=
    {id = 24190, complete = false, received_symbols = 10, packets = std::vector of length 10, capacity 10 = {std::shared_ptr<nimbro_topic_transport::Packet> (use count 1, weak count 0) = {get() = 0x7fffd48a92b0}, std::shared_ptr<nimbro_topic_transport::Packet> (use count 1, weak count 0) = {get() = 0x7fffd48b0120}, std::shared_ptr<nimbro_topic_transport::Packet> (use count 1, weak count 0) = {get() = 0x7fffd48b06f0}, std::shared_ptr<nimbro_topic_transport::Packet> (use count 1, weak count 0) = {get() = 0x7fffd48b0cc0}, std::shared_ptr<nimbro_topic_transport::Packet> (use count 2, weak count 0) = {get() = 0x7fffd48b1290}, std::shared_ptr<nimbro_topic_transport::Packet> (use count 1, weak count 0) = {get() = 0x7fffd48accd0}, std::shared_ptr<nimbro_topic_transport::Packet> (use count 1, weak count 0) = {get() = 0x7fffd48a9e50}, std::shared_ptr<nimbro_topic_transport::Packet> (use count 1, weak count 0) = {get() = 0x7fffd48a8140}, std::shared_ptr<nimbro_topic_transport::Packet> (empty) = {get() = 0x0}, std::shared_ptr<nimbro_topic_transport::Packet> (use count 1, weak count 0) = {get() = 0x7fffd48aefb0}}, decoder = std::shared_ptr<WirehairCodec_t> (empty) = {get() = 0x0}}, packet=...) at /usr/include/c++/9/bits/shared_ptr_base.h:1020
#1  0x00007ffff7d6df79 in nimbro_topic_transport::Depacketizer::addPacket (this=0x7fffffffbfb0,
    packet=std::shared_ptr<nimbro_topic_transport::Packet> (use count 2, weak count 0) = {...})
    at /home/subt/catkin_ws/src/nimbro_network/nimbro_topic_transport/src/receiver/depacketizer.cpp:44
#2  0x00007ffff7d71432 in std::function<void (std::shared_ptr<nimbro_topic_transport::Packet> const&)>::operator()(std::shared_ptr<nimbro_topic_transport::Packet> const&) const (__args#0=std::shared_ptr<nimbro_topic_transport::Packet> (use count 2, weak count 0) = {...},

[xqms]

Hey, thanks for the report! I think you uncovered a pretty serious bug - I'm not sure how this went unnoticed for so long :/

I'm pretty sure it's not a concurrency issue, since Depacketizer::addPacket is protected with a mutex.

But there's a bug hidden in addPacket(): It first saves the iterator, and then calls pruneMessages(), which potentially removes all messages.

Could you please try the fix in #18?

[marco-tranzatto]

@xqms thanks for your quick reply and fix ... amazing!

I did try the proposed fix a few times and could not reproduce the error anymore. I'm gonna try again next week with more extensive tests and let you know.

Once again, thanks a lot for the fix!
Best,
Marco

[marco-tranzatto]

Hi @xqms

Unfortunately the issue happens again even after using the proposed fix. I got another backtrack:

(gdb) bt
#0  0x00007ffff7d6ce72 in nimbro_topic_transport::Depacketizer::handleMessagePacket (
    this=0x7fffffffbed0, it=
    {id = 6619, complete = false, received_symbols = 11, packets = std::vector of length 11, capacity 11 = {std::shared_ptr<nimbro_topic_transport::Packet> (use count 1, weak count 0) = {get() = 0x7fffd420e940}, std::shared_ptr<nimbro_topic_transport::Packet> (use count 1, weak count 0) = {get() = 0x7fffd420e370}, std::shared_ptr<nimbro_topic_transport::Packet> (use count 1, weak count 0) = {get() = 0x7fffd420ef10}, std::shared_ptr<nimbro_topic_transport::Packet> (use count 1, weak count 0) = {get() = 0x7fffd420fab0}, std::shared_ptr<nimbro_topic_transport::Packet> (use count 1, weak count 0) = {get() = 0x7fffd4210650}, std::shared_ptr<nimbro_topic_transport::Packet> (use count 1, weak count 0) = {get() = 0x7fffd42111f0}, std::shared_ptr<nimbro_topic_transport::Packet> (use count 1, weak count 0) = {get() = 0x7fffd4211d90}, std::shared_ptr<nimbro_topic_transport::Packet> (use count 1, weak count 0) = {get() = 0x7fffd4212930}, std::shared_pt

FYI @tomlankhorst

[xqms]

That's unfortunate... Could you please compile nimbro_network in Debug mode (cmake arg -DCMAKE_BUILD_TYPE=Debug) so that we can get a more complete backtrace?