commit 84d360771bb8c73011b5d253344881ed22a78e2d
author Niels Möller <nisse@webrtc.org> Wed Oct 28 13:05:07 2020
committer Commit Bot <commit-bot@chromium.org> Wed Oct 28 15:19:06 2020
tree d5ecb0504f81fcbbfa7956010a8a9e70e3015a9c
parent 2c00c40ff9b5d5286b77ae0fc1fc6b9a614d3d82

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

diff --git a/modules/audio_coding/BUILD.gn b/modules/audio_coding/BUILD.gn
index bcbf9f3..e440b43 100644
--- a/modules/audio_coding/BUILD.gn
+++ b/modules/audio_coding/BUILD.gn
@@ -1390,7 +1390,6 @@
       "../../rtc_base:checks",
       "../../rtc_base:rtc_base_approved",
       "../../rtc_base/synchronization:mutex",
-      "../../rtc_base/synchronization:rw_lock_wrapper",
       "../../test:fileutils",
       "../../test:test_support",
     ]

modules/audio_coding/test/RTPFile.cc

diff --git a/modules/audio_coding/test/RTPFile.cc b/modules/audio_coding/test/RTPFile.cc
index 9681a45..0c44e42 100644
--- a/modules/audio_coding/test/RTPFile.cc
+++ b/modules/audio_coding/test/RTPFile.cc
@@ -80,14 +80,6 @@
   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 @@
                       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 @@
 }
 
 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) {

modules/audio_coding/test/RTPFile.h

diff --git a/modules/audio_coding/test/RTPFile.h b/modules/audio_coding/test/RTPFile.h
index 74fe9e8..a3d1520 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 RTPBuffer : public RTPStream {
  public:
-  RTPBuffer();
+  RTPBuffer() = default;
 
-  ~RTPBuffer();
+  ~RTPBuffer() = default;
 
   void Write(const uint8_t payloadType,
              const uint32_t timeStamp,
@@ -89,8 +90,8 @@
   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 {