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

Fix for intermittent tsan2 errors from SendRtpToRtpOnThread and SendSrtpToSrtpOnThread.

Browse Source 
Changed the channel unittest to use locking when reading/writing the
result variable. To do this, I had to move the result into the thread
object, which in turn required me to properly handle the lifetime of the
thread object, since it cannot disappear while we want to read the
result.

It is still possible to have the result being written to a local
variable, but it will only be updated as the thread object is
destroyed. It is used to for the implementation of
CallOnThreadAndWaitForDone. The old CallOnThread is gone and replaced by
ScopedCallThread instead.

BUG=webrtc:5524

Review URL: https://codereview.webrtc.org/1736763006

Cr-Commit-Position: refs/heads/master@{#12027}
This commit is contained in:
ossu
2016-03-17 02:31:13 -07:00
committed by Commit bot
parent 8300641472
commit 292d658b20
2 changed files with 66 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
webrtc/build/sanitizers/tsan_suppressions_webrtc.cc
View File

@ -92,11 +92,6 @@ char kTSanDefaultSuppressions[] =
// https://code.google.com/p/libyuv/issues/detail?id=508
"race:InitCpuFlags\n"
// https://bugs.chromium.org/p/webrtc/issues/detail?id=5524
"race:AudioChannelTest_SendSrtpToSrtpOnThread_Test::TestBody\n"
"race:VideoChannelTest_SendSrtpToSrtpOnThread_Test::TestBody\n"
"race:DataChannelTest_SendSrtpToSrtpOnThread_Test::TestBody\n"
// End of suppressions.
; // Please keep this semicolon.

98
webrtc/pc/channel_unittest.cc
View File

@ -11,6 +11,7 @@
#include <memory>
#include "webrtc/base/arraysize.h"
#include "webrtc/base/criticalsection.h"
#include "webrtc/base/fileutils.h"
#include "webrtc/base/gunit.h"
#include "webrtc/base/helpers.h"
@ -410,28 +411,64 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> {
class CallThread : public rtc::SignalThread {
public:
typedef bool (ChannelTest<T>::*Method)();
CallThread(ChannelTest<T>* obj, Method method, bool* result)
CallThread(ChannelTest<T>* obj, Method method, bool* result = nullptr)
: obj_(obj),
method_(method),
result_(result) {
*result = false;
result_(false),
result_ptr_(result) {
if (result_ptr_)
*result_ptr_ = false;
}
virtual void DoWork() {
bool result = (*obj_.*method_)();
if (result_) {
*result_ = result;
~CallThread() {
if (result_ptr_) {
rtc::CritScope cs(&result_lock_);
*result_ptr_ = result_;
}
}
virtual void DoWork() {
SetResult((*obj_.*method_)());
}
bool result() {
rtc::CritScope cs(&result_lock_);
return result_;
}
private:
void SetResult(const bool& result) {
rtc::CritScope cs(&result_lock_);
result_ = result;
}
ChannelTest<T>* obj_;
Method method_;
bool* result_;
rtc::CriticalSection result_lock_;
bool result_ GUARDED_BY(result_lock_);
bool* result_ptr_;
};
// Will manage the lifetime of a CallThread, making sure it's
// destroyed before this object goes out of scope.
class ScopedCallThread {
public:
using Method = typename CallThread::Method;
ScopedCallThread(ChannelTest<T>* obj, Method method)
: thread_(new CallThread(obj, method)) {
thread_->Start();
}
~ScopedCallThread() {
thread_->Destroy(true);
}
bool result() const { return thread_->result(); }
private:
CallThread* thread_;
};
void CallOnThread(typename CallThread::Method method, bool* result) {
CallThread* thread = new CallThread(this, method, result);
thread->Start();
thread->Release();
}
void CallOnThreadAndWaitForDone(typename CallThread::Method method,
bool* result) {
@ -1328,48 +1365,46 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> {
// Test that we properly send RTP without SRTP from a thread.
void SendRtpToRtpOnThread() {
bool sent_rtp1, sent_rtp2, sent_rtcp1, sent_rtcp2;
CreateChannels(RTCP, RTCP);
EXPECT_TRUE(SendInitiate());
EXPECT_TRUE(SendAccept());
CallOnThread(&ChannelTest<T>::SendRtp1, &sent_rtp1);
CallOnThread(&ChannelTest<T>::SendRtp2, &sent_rtp2);
CallOnThread(&ChannelTest<T>::SendRtcp1, &sent_rtcp1);
CallOnThread(&ChannelTest<T>::SendRtcp2, &sent_rtcp2);
ScopedCallThread send_rtp1(this, &ChannelTest<T>::SendRtp1);
ScopedCallThread send_rtp2(this, &ChannelTest<T>::SendRtp2);
ScopedCallThread send_rtcp1(this, &ChannelTest<T>::SendRtcp1);
ScopedCallThread send_rtcp2(this, &ChannelTest<T>::SendRtcp2);
EXPECT_TRUE_WAIT(CheckRtp1(), 1000);
EXPECT_TRUE_WAIT(CheckRtp2(), 1000);
EXPECT_TRUE_WAIT(sent_rtp1, 1000);
EXPECT_TRUE_WAIT(sent_rtp2, 1000);
EXPECT_TRUE_WAIT(send_rtp1.result(), 1000);
EXPECT_TRUE_WAIT(send_rtp2.result(), 1000);
EXPECT_TRUE(CheckNoRtp1());
EXPECT_TRUE(CheckNoRtp2());
EXPECT_TRUE_WAIT(CheckRtcp1(), 1000);
EXPECT_TRUE_WAIT(CheckRtcp2(), 1000);
EXPECT_TRUE_WAIT(sent_rtcp1, 1000);
EXPECT_TRUE_WAIT(sent_rtcp2, 1000);
EXPECT_TRUE_WAIT(send_rtcp1.result(), 1000);
EXPECT_TRUE_WAIT(send_rtcp2.result(), 1000);
EXPECT_TRUE(CheckNoRtcp1());
EXPECT_TRUE(CheckNoRtcp2());
}
// Test that we properly send SRTP with RTCP from a thread.
void SendSrtpToSrtpOnThread() {
bool sent_rtp1, sent_rtp2, sent_rtcp1, sent_rtcp2;
CreateChannels(RTCP | SECURE, RTCP | SECURE);
EXPECT_TRUE(SendInitiate());
EXPECT_TRUE(SendAccept());
CallOnThread(&ChannelTest<T>::SendRtp1, &sent_rtp1);
CallOnThread(&ChannelTest<T>::SendRtp2, &sent_rtp2);
CallOnThread(&ChannelTest<T>::SendRtcp1, &sent_rtcp1);
CallOnThread(&ChannelTest<T>::SendRtcp2, &sent_rtcp2);
ScopedCallThread send_rtp1(this, &ChannelTest<T>::SendRtp1);
ScopedCallThread send_rtp2(this, &ChannelTest<T>::SendRtp2);
ScopedCallThread send_rtcp1(this, &ChannelTest<T>::SendRtcp1);
ScopedCallThread send_rtcp2(this, &ChannelTest<T>::SendRtcp2);
EXPECT_TRUE_WAIT(CheckRtp1(), 1000);
EXPECT_TRUE_WAIT(CheckRtp2(), 1000);
EXPECT_TRUE_WAIT(sent_rtp1, 1000);
EXPECT_TRUE_WAIT(sent_rtp2, 1000);
EXPECT_TRUE_WAIT(send_rtp1.result(), 1000);
EXPECT_TRUE_WAIT(send_rtp2.result(), 1000);
EXPECT_TRUE(CheckNoRtp1());
EXPECT_TRUE(CheckNoRtp2());
EXPECT_TRUE_WAIT(CheckRtcp1(), 1000);
EXPECT_TRUE_WAIT(CheckRtcp2(), 1000);
EXPECT_TRUE_WAIT(sent_rtcp1, 1000);
EXPECT_TRUE_WAIT(sent_rtcp2, 1000);
EXPECT_TRUE_WAIT(send_rtcp1.result(), 1000);
EXPECT_TRUE_WAIT(send_rtcp2.result(), 1000);
EXPECT_TRUE(CheckNoRtcp1());
EXPECT_TRUE(CheckNoRtcp2());
}
@ -1627,7 +1662,6 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> {
void TestFlushRtcp() {
bool send_rtcp1;
CreateChannels(RTCP, RTCP);
EXPECT_TRUE(SendInitiate());
EXPECT_TRUE(SendAccept());