(ABORT) RTC::RTCP::FeedbackRtpTransport::AddPacket() | failed assertion `baseSet': base not set

[ibc]

• Operating system: Linux Debian
• mediasoup version:3.13.19

Issue description

Logs:

2024-02-20T07:08:16.181Z mediasoup:ERROR:Worker (stderr) (ABORT) RTC::RTCP::FeedbackRtpTransport::AddPacket() | failed assertion `baseSet': base not set
2024-02-20T07:08:16.569Z mediasoup:ERROR:Channel Producer Channel error: Error: read ECONNRESET
2024-02-20T07:08:16.574Z mediasoup-demo-server:WARN:Room _createDataConsumer() | failed:Error: request timeout at Timeout._onTimeout (/var/www/v3demo.mediasoup.org/node_modules/protoo-server/lib/Peer.js:156:14) at listOnTimeout (node:internal/timers:569:17) at process.processTimers (node:internal/timers:512:7)
2024-02-20T07:08:16.784Z mediasoup:ERROR:Worker worker process died unexpectedly [pid:1695953, code:null, signal:SIGABRT]
2024-02-20T07:08:16.823Z mediasoup-demo-server:ERROR mediasoup Worker died, exiting  in 2 seconds... [pid:1695953]

Core dump:

Program terminated with signal SIGABRT, Aborted.
#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
44  ./nptl/pthread_kill.c: No such file or directory.
[Current thread is 1 (Thread 0x7f0774b3e440 (LWP 1695953))]
(gdb) bt
#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
#1  0x00007f0774bcad9f in __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
#2  0x00007f0774b7bf32 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x00007f0774b66472 in __GI_abort () at ./stdlib/abort.c:79
#4  0x000055b064e4bd6e in RTC::RTCP::FeedbackRtpTransportPacket::AddPacket(unsigned short, unsigned long, unsigned long) ()
#5  0x000055b064dd9ade in RTC::TransportCongestionControlServer::FillAndSendTransportCcFeedback() ()
#6  0x000055b0650dcf3e in uv.run_timers ()
#7  0x000055b0650e0c88 in uv_run ()
#8  0x000055b064c9a849 in DepLibUV::RunLoop() ()
#9  0x000055b064ca4d62 in Worker::Worker(Channel::ChannelSocket*) ()
#10 0x000055b064c998e4 in mediasoup_worker_run ()
#11 0x000055b064c98a2c in main ()

Clearly a regression in PR https://github.com/versatica/mediasoup/pull/1088 " Make transport-cc feedback work similarly to libwebrtc".

[ibc]

CC @penguinol

[ibc]

I strongly fail to see how this crash can happen:

• In TransportCongestionControlServer there is a timer that invokes its FillAndSendTransportCcFeedback().

• FillAndSendTransportCcFeedback() is the only method that calls FeedbackRtpTransportPacket::SetBase() and FeedbackRtpTransportPacket::AddPacket() and it always calls the former first:

  void TransportCongestionControlServer::FillAndSendTransportCcFeedback()
  {
      MS_TRACE();
  
      if (!this->transportWideSeqNumberReceived)
      {
          return;
      }
  
      auto it = this->mapPacketArrivalTimes.lower_bound(this->transportCcFeedbackWideSeqNumStart);
  
      if (it == this->mapPacketArrivalTimes.end())
      {
          return;
      }
  
      // Set base sequence num and reference time.
      this->transportCcFeedbackPacket->SetBase(this->transportCcFeedbackWideSeqNumStart, it->second);
  
      for (; it != this->mapPacketArrivalTimes.end(); ++it)
      {
          auto result =
            this->transportCcFeedbackPacket->AddPacket(it->first, it->second, this->maxRtcpPacketLen);

• So it's impossible that FeedbackRtpTransportPacket::AddPacket() is called before FeedbackRtpTransportPacket::SetBase() is called, so this->baseSet is always true when FeedbackRtpTransportPacket::AddPacket() and hence the assertion should not fail:

  MS_ASSERT(this->baseSet, "base not set");

• And nowhere else this->baseSet in FeedbackRtpTransportPacket class is reset to false.

[ibc]

OPPPPS, HERE!

void TransportCongestionControlServer::TransportDisconnected()
    {
        MS_TRACE();

        switch (this->bweType)
        {
            case RTC::BweType::TRANSPORT_CC:
            {
                this->transportCcFeedbackSendPeriodicTimer->Stop();

                // Create a new feedback packet.
                this->transportCcFeedbackPacket.reset(new RTC::RTCP::FeedbackRtpTransportPacket(0u, 0u));

                break;
            }

            default:;
        }
    }

Here we are overriding this->transportCcFeedbackPacket with a fresh packet so its baseSet property is of course false. However we are also stopping the timer so still I don't understand how the issue can happen.

[ibc]

Oh... we are reseting the packet in many other places!!!!! here the bug!!!

                case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::MAX_SIZE_EXCEEDED:
                {
                    // This should not happen.
                    MS_WARN_DEV("transport-cc feedback packet is exceeded");

                    // Create a new feedback packet.
                    this->transportCcFeedbackPacket.reset(new RTC::RTCP::FeedbackRtpTransportPacket(
                      this->transportCcFeedbackSenderSsrc, this->transportCcFeedbackMediaSsrc));
                }

                case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::FATAL:
                {
                    // Create a new feedback packet.
                    this->transportCcFeedbackPacket.reset(new RTC::RTCP::FeedbackRtpTransportPacket(
                      this->transportCcFeedbackSenderSsrc, this->transportCcFeedbackMediaSsrc));

                    // Use current packet count.
                    // NOTE: Do not increment it since the previous ongoing feedback
                    // packet was not sent.
                    this->transportCcFeedbackPacket->SetFeedbackPacketCount(
                      this->transportCcFeedbackPacketCount);

                    break;
                }
            }