zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix locking in RTPFile class

Browse Source 
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}
This commit is contained in:
Niels Möller
2020-10-28 14:05:07 +01:00
committed by Commit Bot
parent 2c00c40ff9
commit 84d360771b
3 changed files with 15 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
modules/audio_coding/BUILD.gn
View File

@ -1390,7 +1390,6 @@ if (rtc_include_tests) {
"../../rtc_base:checks", "../../rtc_base:checks",
"../../rtc_base:rtc_base_approved", "../../rtc_base:rtc_base_approved",
"../../rtc_base/synchronization:mutex", "../../rtc_base/synchronization:mutex",
"../../rtc_base/synchronization:rw_lock_wrapper",
"../../test:fileutils", "../../test:fileutils",
"../../test:test_support", "../../test:test_support",
] ]

27
modules/audio_coding/test/RTPFile.cc
View File

@ -80,14 +80,6 @@ RTPPacket::~RTPPacket() {
delete[] payloadData; delete[] payloadData;
} }
RTPBuffer::RTPBuffer() {
_queueRWLock = RWLockWrapper::CreateRWLock();
}
RTPBuffer::~RTPBuffer() {
delete _queueRWLock;
}
void RTPBuffer::Write(const uint8_t payloadType, void RTPBuffer::Write(const uint8_t payloadType,
const uint32_t timeStamp, const uint32_t timeStamp,
const int16_t seqNo, const int16_t seqNo,
@ -96,19 +88,20 @@ void RTPBuffer::Write(const uint8_t payloadType,
uint32_t frequency) { uint32_t frequency) {
RTPPacket* packet = new RTPPacket(payloadType, timeStamp, seqNo, payloadData, RTPPacket* packet = new RTPPacket(payloadType, timeStamp, seqNo, payloadData,
payloadSize, frequency); payloadSize, frequency);
_queueRWLock->AcquireLockExclusive(); MutexLock lock(&mutex_);
_rtpQueue.push(packet); _rtpQueue.push(packet);
_queueRWLock->ReleaseLockExclusive();
} }
size_t RTPBuffer::Read(RTPHeader* rtp_header, size_t RTPBuffer::Read(RTPHeader* rtp_header,
uint8_t* payloadData, uint8_t* payloadData,
size_t payloadSize, size_t payloadSize,
uint32_t* offset) { uint32_t* offset) {
_queueRWLock->AcquireLockShared(); RTPPacket* packet;
RTPPacket* packet = _rtpQueue.front(); {
_rtpQueue.pop(); MutexLock lock(&mutex_);
_queueRWLock->ReleaseLockShared(); packet = _rtpQueue.front();
_rtpQueue.pop();
}
rtp_header->markerBit = 1; rtp_header->markerBit = 1;
rtp_header->payloadType = packet->payloadType; rtp_header->payloadType = packet->payloadType;
rtp_header->sequenceNumber = packet->seqNo; rtp_header->sequenceNumber = packet->seqNo;
@ -125,10 +118,8 @@ size_t RTPBuffer::Read(RTPHeader* rtp_header,
} }
bool RTPBuffer::EndOfFile() const { bool RTPBuffer::EndOfFile() const {
_queueRWLock->AcquireLockShared(); MutexLock lock(&mutex_);
bool eof = _rtpQueue.empty(); return _rtpQueue.empty();
_queueRWLock->ReleaseLockShared();
return eof;
} }
void RTPFile::Open(const char* filename, const char* mode) { void RTPFile::Open(const char* filename, const char* mode) {

11
modules/audio_coding/test/RTPFile.h
View File

@ -16,7 +16,8 @@
#include <queue> #include <queue>
#include "api/rtp_headers.h" #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 { namespace webrtc {
@ -70,9 +71,9 @@ class RTPPacket {
class RTPBuffer : public RTPStream { class RTPBuffer : public RTPStream {
public: public:
RTPBuffer(); RTPBuffer() = default;
~RTPBuffer(); ~RTPBuffer() = default;
void Write(const uint8_t payloadType, void Write(const uint8_t payloadType,
const uint32_t timeStamp, const uint32_t timeStamp,
@ -89,8 +90,8 @@ class RTPBuffer : public RTPStream {
bool EndOfFile() const override; bool EndOfFile() const override;
private: private:
RWLockWrapper* _queueRWLock; mutable Mutex mutex_;
std::queue<RTPPacket*> _rtpQueue; std::queue<RTPPacket*> _rtpQueue RTC_GUARDED_BY(&mutex_);
}; };
class RTPFile : public RTPStream { class RTPFile : public RTPStream {