From 3eac1111151fcda7b40e7994d4a0f45ac08ccbbc Mon Sep 17 00:00:00 2001
From: Markus Handell <handellm@webrtc.org>
Date: Wed, 27 May 2020 17:08:41 +0200
Subject: [PATCH] PacketBuffer: remove lock recursions.

This change removes lock recursions and adds thread annotations.

Bug: webrtc:11567
Change-Id: Ibc429571926693f4b3de78f97a5dc5501d93a4a3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176240
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Commit-Queue: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31369}
---
 modules/video_coding/packet_buffer.cc | 26 +++++++++++++++-----------
 modules/video_coding/packet_buffer.h  | 20 +++++++++++++-------
 2 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/modules/video_coding/packet_buffer.cc b/modules/video_coding/packet_buffer.cc
index 5db3c0f670..d62dc84cd3 100644
--- a/modules/video_coding/packet_buffer.cc
+++ b/modules/video_coding/packet_buffer.cc
@@ -112,7 +112,7 @@ PacketBuffer::InsertResult PacketBuffer::InsertPacket(
       // Clear the buffer, delete payload, and return false to signal that a
       // new keyframe is needed.
       RTC_LOG(LS_WARNING) << "Clear PacketBuffer and request key frame.";
-      Clear();
+      ClearInternal();
       result.buffer_cleared = true;
       return result;
     }
@@ -174,16 +174,7 @@ void PacketBuffer::ClearTo(uint16_t seq_num) {
 
 void PacketBuffer::Clear() {
   rtc::CritScope lock(&crit_);
-  for (auto& entry : buffer_) {
-    entry = nullptr;
-  }
-
-  first_packet_received_ = false;
-  is_cleared_to_first_seq_num_ = false;
-  last_received_packet_ms_.reset();
-  last_received_keyframe_packet_ms_.reset();
-  newest_inserted_seq_num_.reset();
-  missing_packets_.clear();
+  ClearInternal();
 }
 
 PacketBuffer::InsertResult PacketBuffer::InsertPadding(uint16_t seq_num) {
@@ -204,6 +195,19 @@ absl::optional<int64_t> PacketBuffer::LastReceivedKeyframePacketMs() const {
   return last_received_keyframe_packet_ms_;
 }
 
+void PacketBuffer::ClearInternal() {
+  for (auto& entry : buffer_) {
+    entry = nullptr;
+  }
+
+  first_packet_received_ = false;
+  is_cleared_to_first_seq_num_ = false;
+  last_received_packet_ms_.reset();
+  last_received_keyframe_packet_ms_.reset();
+  newest_inserted_seq_num_.reset();
+  missing_packets_.clear();
+}
+
 bool PacketBuffer::ExpandBufferSize() {
   if (buffer_.size() == max_size_) {
     RTC_LOG(LS_WARNING) << "PacketBuffer is already at max size (" << max_size_
diff --git a/modules/video_coding/packet_buffer.h b/modules/video_coding/packet_buffer.h
index c480e37239..e4dc237a1e 100644
--- a/modules/video_coding/packet_buffer.h
+++ b/modules/video_coding/packet_buffer.h
@@ -82,19 +82,25 @@ class PacketBuffer {
   PacketBuffer(Clock* clock, size_t start_buffer_size, size_t max_buffer_size);
   ~PacketBuffer();
 
-  InsertResult InsertPacket(std::unique_ptr<Packet> packet)
-      ABSL_MUST_USE_RESULT;
-  InsertResult InsertPadding(uint16_t seq_num) ABSL_MUST_USE_RESULT;
-  void ClearTo(uint16_t seq_num);
-  void Clear();
+  InsertResult InsertPacket(std::unique_ptr<Packet> packet) ABSL_MUST_USE_RESULT
+      RTC_LOCKS_EXCLUDED(crit_);
+  InsertResult InsertPadding(uint16_t seq_num) ABSL_MUST_USE_RESULT
+      RTC_LOCKS_EXCLUDED(crit_);
+  void ClearTo(uint16_t seq_num) RTC_LOCKS_EXCLUDED(crit_);
+  void Clear() RTC_LOCKS_EXCLUDED(crit_);
 
   // Timestamp (not RTP timestamp) of the last received packet/keyframe packet.
-  absl::optional<int64_t> LastReceivedPacketMs() const;
-  absl::optional<int64_t> LastReceivedKeyframePacketMs() const;
+  absl::optional<int64_t> LastReceivedPacketMs() const
+      RTC_LOCKS_EXCLUDED(crit_);
+  absl::optional<int64_t> LastReceivedKeyframePacketMs() const
+      RTC_LOCKS_EXCLUDED(crit_);
 
  private:
   Clock* const clock_;
 
+  // Clears with |crit_| taken.
+  void ClearInternal() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
+
   // Tries to expand the buffer.
   bool ExpandBufferSize() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);