From 84d360771bb8c73011b5d253344881ed22a78e2d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Niels=20M=C3=B6ller?= <nisse@webrtc.org>
Date: Wed, 28 Oct 2020 14:05:07 +0100
Subject: [PATCH] Fix locking in RTPFile class

This code used to have a reader-writer lock, and call
std::queue::pop() with only a reader lock, which appears unsafe. Code
changed to use a plain webrtc::Mutex.

Bug: webrtc:12102
Change-Id: Icbea17a824c91975dfebd4d05bbd0c21e1abeadc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/190700
Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32511}
---
 modules/audio_coding/BUILD.gn        |  1 -
 modules/audio_coding/test/RTPFile.cc | 27 +++++++++------------------
 modules/audio_coding/test/RTPFile.h  | 11 ++++++-----
 3 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/modules/audio_coding/BUILD.gn b/modules/audio_coding/BUILD.gn
index bcbf9f37bb..e440b43da7 100644
--- a/modules/audio_coding/BUILD.gn
+++ b/modules/audio_coding/BUILD.gn
@@ -1390,7 +1390,6 @@ if (rtc_include_tests) {
       "../../rtc_base:checks",
       "../../rtc_base:rtc_base_approved",
       "../../rtc_base/synchronization:mutex",
-      "../../rtc_base/synchronization:rw_lock_wrapper",
       "../../test:fileutils",
       "../../test:test_support",
     ]
diff --git a/modules/audio_coding/test/RTPFile.cc b/modules/audio_coding/test/RTPFile.cc
index 9681a453d9..0c44e4298c 100644
--- a/modules/audio_coding/test/RTPFile.cc
+++ b/modules/audio_coding/test/RTPFile.cc
@@ -80,14 +80,6 @@ RTPPacket::~RTPPacket() {
   delete[] payloadData;
 }
 
-RTPBuffer::RTPBuffer() {
-  _queueRWLock = RWLockWrapper::CreateRWLock();
-}
-
-RTPBuffer::~RTPBuffer() {
-  delete _queueRWLock;
-}
-
 void RTPBuffer::Write(const uint8_t payloadType,
                       const uint32_t timeStamp,
                       const int16_t seqNo,
@@ -96,19 +88,20 @@ void RTPBuffer::Write(const uint8_t payloadType,
                       uint32_t frequency) {
   RTPPacket* packet = new RTPPacket(payloadType, timeStamp, seqNo, payloadData,
                                     payloadSize, frequency);
-  _queueRWLock->AcquireLockExclusive();
+  MutexLock lock(&mutex_);
   _rtpQueue.push(packet);
-  _queueRWLock->ReleaseLockExclusive();
 }
 
 size_t RTPBuffer::Read(RTPHeader* rtp_header,
                        uint8_t* payloadData,
                        size_t payloadSize,
                        uint32_t* offset) {
-  _queueRWLock->AcquireLockShared();
-  RTPPacket* packet = _rtpQueue.front();
-  _rtpQueue.pop();
-  _queueRWLock->ReleaseLockShared();
+  RTPPacket* packet;
+  {
+    MutexLock lock(&mutex_);
+    packet = _rtpQueue.front();
+    _rtpQueue.pop();
+  }
   rtp_header->markerBit = 1;
   rtp_header->payloadType = packet->payloadType;
   rtp_header->sequenceNumber = packet->seqNo;
@@ -125,10 +118,8 @@ size_t RTPBuffer::Read(RTPHeader* rtp_header,
 }
 
 bool RTPBuffer::EndOfFile() const {
-  _queueRWLock->AcquireLockShared();
-  bool eof = _rtpQueue.empty();
-  _queueRWLock->ReleaseLockShared();
-  return eof;
+  MutexLock lock(&mutex_);
+  return _rtpQueue.empty();
 }
 
 void RTPFile::Open(const char* filename, const char* mode) {
diff --git a/modules/audio_coding/test/RTPFile.h b/modules/audio_coding/test/RTPFile.h
index 74fe9e8013..a3d1520922 100644
--- a/modules/audio_coding/test/RTPFile.h
+++ b/modules/audio_coding/test/RTPFile.h
@@ -16,7 +16,8 @@
 #include <queue>
 
 #include "api/rtp_headers.h"
-#include "rtc_base/synchronization/rw_lock_wrapper.h"
+#include "rtc_base/synchronization/mutex.h"
+#include "rtc_base/thread_annotations.h"
 
 namespace webrtc {
 
@@ -70,9 +71,9 @@ class RTPPacket {
 
 class RTPBuffer : public RTPStream {
  public:
-  RTPBuffer();
+  RTPBuffer() = default;
 
-  ~RTPBuffer();
+  ~RTPBuffer() = default;
 
   void Write(const uint8_t payloadType,
              const uint32_t timeStamp,
@@ -89,8 +90,8 @@ class RTPBuffer : public RTPStream {
   bool EndOfFile() const override;
 
  private:
-  RWLockWrapper* _queueRWLock;
-  std::queue<RTPPacket*> _rtpQueue;
+  mutable Mutex mutex_;
+  std::queue<RTPPacket*> _rtpQueue RTC_GUARDED_BY(&mutex_);
 };
 
 class RTPFile : public RTPStream {