[M148] Fix TOCTOU race in RtpVideoSender::OnVideoLayersAllocationUpdated RtpVideoSender::OnVideoLayersAllocationUpdated runs on the encoder thread, checks if it's active, and then posts a task to the transport queue to update module status. If SetSending(false) is called on the transport queue before the posted task runs, the task would previously re-register modules in PacketRouter even though active_ was false. This lead to dangling pointers in PacketRouter when the RtpVideoSender was destroyed. This CL adds an IsActive() check inside the posted task to ensure we don't re-register modules if the sender has been deactivated. (cherry picked from commit 77d265670fa7d7f358c86c54704a6e97c27a6005) Bug: chromium:500767595 Test: RtpVideoSenderTest.PostTaskRaceDoesNotLeadToDanglingPointer Change-Id: I18827c34bece7f5ebc41d159d9e19e356afbc404 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/462880 Commit-Queue: Harald Alvestrand <hta@webrtc.org> Reviewed-by: Erik Språng <sprang@webrtc.org> Cr-Original-Commit-Position: refs/heads/main@{#47384} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/465961 Cr-Commit-Position: refs/branch-heads/7778@{#5} Cr-Branched-From: ca896b7ffef011bbf6957c99d413c5aac602c99f-refs/heads/main@{#47319}

diff --git a/call/BUILD.gn b/call/BUILD.gn
index 815d28a..a784332 100644
--- a/call/BUILD.gn
+++ b/call/BUILD.gn

@@ -550,6 +550,7 @@
         "../api/video:video_codec_constants",
         "../api/video:video_frame",
         "../api/video:video_frame_type",
+        "../api/video:video_layers_allocation",
         "../api/video:video_rtp_headers",
         "../api/video_codecs:video_codecs_api",
         "../audio",

diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 73e8898..7267add 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc

@@ -695,6 +695,11 @@
     transport_queue_.PostTask(
         SafeTask(safety_.flag(), [this, sending = std::move(sending)] {
           RTC_DCHECK_RUN_ON(&transport_checker_);
+          // It's possible for another task to be scheduled on the transport
+          // checker ahead of this call that makes the sender not active.
+          if (!IsActive()) {
+            return;
+          }
           RTC_CHECK_EQ(sending.size(), rtp_streams_.size());
           for (size_t i = 0; i < sending.size(); ++i) {
             SetModuleIsActive(sending[i], *rtp_streams_[i].rtp_rtcp);

diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc
index a979e05..d7d30df 100644
--- a/call/rtp_video_sender_unittest.cc
+++ b/call/rtp_video_sender_unittest.cc

@@ -40,6 +40,7 @@
 #include "api/video/encoded_image.h"
 #include "api/video/video_codec_type.h"
 #include "api/video/video_frame_type.h"
+#include "api/video/video_layers_allocation.h"
 #include "api/video_codecs/video_encoder.h"
 #include "call/rtp_config.h"
 #include "call/rtp_transport_config.h"
@@ -1607,4 +1608,73 @@
   EXPECT_EQ(retransmitted_rtp_sequence_numbers, base_rtp_sequence_numbers);
 }
 
+TEST(RtpVideoSenderTest, PostTaskRaceDoesNotLeadToDanglingPointer) {
+  NiceMock<MockTransport> transport;
+  NiceMock<MockRtcpIntraFrameObserver> encoder_feedback;
+
+  GlobalSimulatedTimeController time_controller(Timestamp::Millis(1000000));
+  Environment env = CreateEnvironment(time_controller.GetClock(),
+                                      time_controller.CreateTaskQueueFactory());
+
+  VideoSendStream::Config config(&transport);
+  config.rtp.ssrcs = {kSsrc1};
+
+  SendStatisticsProxy stats_proxy(
+      time_controller.GetClock(), config,
+      VideoEncoderConfig::ContentType::kRealtimeVideo, env.field_trials());
+
+  BitrateConstraints bitrate_config = GetBitrateConfig();
+  RtpTransportConfig transport_config{.env = env,
+                                      .bitrate_config = bitrate_config};
+  RtpTransportControllerSend transport_controller(transport_config);
+  transport_controller.EnsureStarted();
+
+  RateLimiter retransmission_rate_limiter(time_controller.GetClock(),
+                                          kRetransmitWindowSizeMs);
+
+  std::map<uint32_t, RtpState> suspended_ssrcs;
+  std::map<uint32_t, RtpPayloadState> suspended_payload_states;
+
+  auto router = std::make_unique<RtpVideoSender>(
+      env, time_controller.GetMainThread(), suspended_ssrcs,
+      suspended_payload_states, config.rtp, config.rtcp_report_interval_ms,
+      &transport,
+      CreateObservers(&encoder_feedback, &stats_proxy, &stats_proxy,
+                      &stats_proxy, nullptr, &stats_proxy),
+      &transport_controller, &retransmission_rate_limiter,
+      std::make_unique<FecControllerDefault>(env), nullptr, CryptoOptions{},
+      nullptr);
+
+  router->SetSending(true);
+  // Verify it's registered
+  EXPECT_TRUE(
+      transport_controller.packet_router()->SsrcOfFirstSender().has_value());
+
+  // Trigger race:
+  // 1. OnVideoLayersAllocationUpdated posts a task to re-register modules
+  // because it sees active_ is true.
+  VideoLayersAllocation allocation;
+  allocation.active_spatial_layers.push_back({.rtp_stream_index = 0});
+  router->OnVideoLayersAllocationUpdated(allocation);
+
+  // 2. Immediately set active_ to false. This happens BEFORE the posted task
+  // runs.
+  router->SetSending(false);
+
+  // 3. Let the posted task run. With the fix, it should return early and not
+  // re-register the module because active_ is now false.
+  time_controller.AdvanceTime(TimeDelta::Zero());
+
+  // 4. Verify that PacketRouter does NOT have a sender.
+  EXPECT_FALSE(
+      transport_controller.packet_router()->SsrcOfFirstSender().has_value());
+
+  // 5. Destroy the router.
+  router.reset();
+
+  // 6. Verify that PacketRouter still does not have a sender.
+  EXPECT_FALSE(
+      transport_controller.packet_router()->SsrcOfFirstSender().has_value());
+}
+
 }  // namespace webrtc