Commit 6bfff67c authored by Henrik Boström's avatar Henrik Boström Committed by Commit Bot

Track adapter map stress test added and invalid thread checks removed.

WebRtcMediaStreamTrackAdapterMapStressTest added due to
https://crbug.com/813574.

This continuously creates and destroys local and remote track adapters
concurrently in an attempt to cause a deadlock if possible (hopefully
not).

This also removes some invalid thread checks at the destructor of
~WebRtcMediaStreamTrackAdapter and ~WebRtcMediaStreamAdapterMap. The
first of which made the stress test flaky (https://crbug.com/826365).

The adapter's DCHECK was invalid because the remote audio track's
Dispose (which properly DCHECKs that it is invoked on the main thread)
can jump to the signaling thread and back with a reference to the
adapter. It is possible for the signaling thread's task to be the last
reference to the adapter.

Similarly, WebRtcMediaStreamAdapterMap is reference counted and tasks
referencing on different threads could make which thread references it
last racy. We don't care which thread was the last reference as long as
it is empty (which it is if and only if all adapters have been
destroyed on the appropriate threads).

Bug: 827450
Change-Id: I5f819948fc2148e810f343e027fa8011b03d2335
Reviewed-on: https://chromium-review.googlesource.com/983553Reviewed-by: default avatarGuido Urdaneta <guidou@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551413}
parent e0ea06e3
...@@ -77,8 +77,8 @@ WebRtcMediaStreamAdapterMap::WebRtcMediaStreamAdapterMap( ...@@ -77,8 +77,8 @@ WebRtcMediaStreamAdapterMap::WebRtcMediaStreamAdapterMap(
} }
WebRtcMediaStreamAdapterMap::~WebRtcMediaStreamAdapterMap() { WebRtcMediaStreamAdapterMap::~WebRtcMediaStreamAdapterMap() {
DCHECK(main_thread_->BelongsToCurrentThread());
DCHECK(local_stream_adapters_.empty()); DCHECK(local_stream_adapters_.empty());
DCHECK(remote_stream_adapters_.empty());
} }
std::unique_ptr<WebRtcMediaStreamAdapterMap::AdapterRef> std::unique_ptr<WebRtcMediaStreamAdapterMap::AdapterRef>
......
...@@ -69,7 +69,6 @@ WebRtcMediaStreamTrackAdapter::WebRtcMediaStreamTrackAdapter( ...@@ -69,7 +69,6 @@ WebRtcMediaStreamTrackAdapter::WebRtcMediaStreamTrackAdapter(
} }
WebRtcMediaStreamTrackAdapter::~WebRtcMediaStreamTrackAdapter() { WebRtcMediaStreamTrackAdapter::~WebRtcMediaStreamTrackAdapter() {
DCHECK(main_thread_->BelongsToCurrentThread());
DCHECK(!remote_track_can_complete_initialization_.IsSignaled()); DCHECK(!remote_track_can_complete_initialization_.IsSignaled());
DCHECK(!is_initialized_); DCHECK(!is_initialized_);
} }
......
...@@ -215,4 +215,99 @@ TEST_F(WebRtcMediaStreamTrackAdapterMapTest, GetMissingRemoteTrackAdapter) { ...@@ -215,4 +215,99 @@ TEST_F(WebRtcMediaStreamTrackAdapterMapTest, GetMissingRemoteTrackAdapter) {
EXPECT_EQ(nullptr, map_->GetRemoteTrackAdapter(webrtc_track.get())); EXPECT_EQ(nullptr, map_->GetRemoteTrackAdapter(webrtc_track.get()));
} }
// Continuously calls GetOrCreateLocalTrackAdapter() on the main thread and
// GetOrCreateRemoteTrackAdapter() on the signaling thread hoping to hit
// deadlocks if the operations were to synchronize with the other thread while
// holding the lock.
//
// Note that this deadlock has been notoriously difficult to reproduce. This
// test is added as an attempt to guard against this type of regression, but do
// not trust that if this test passes there is no risk of deadlock.
class WebRtcMediaStreamTrackAdapterMapStressTest
: public WebRtcMediaStreamTrackAdapterMapTest {
public:
WebRtcMediaStreamTrackAdapterMapStressTest()
: WebRtcMediaStreamTrackAdapterMapTest(), remaining_iterations_(0u) {}
void RunStressTest(size_t iterations) {
base::RunLoop run_loop;
remaining_iterations_ = iterations;
PostSignalingThreadLoop();
MainThreadLoop(&run_loop);
run_loop.Run();
// The run loop ensures all operations have began executing, but does not
// guarantee that all of them are complete, i.e. that track adapters have
// been fully initialized and subequently disposed. For that we need to run
// until idle or else we may tear down the test prematurely.
RunMessageLoopsUntilIdle();
}
void MainThreadLoop(base::RunLoop* run_loop) {
for (size_t i = 0u; i < 5u; ++i) {
map_->GetOrCreateLocalTrackAdapter(CreateLocalTrack("local_track_id"));
}
if (--remaining_iterations_ > 0) {
PostSignalingThreadLoop();
PostMainThreadLoop(run_loop);
} else {
// We are now done, but there may still be operations pending to execute
// on signaling thread so we perform Quit() in a post to the signaling
// thread. This ensures that Quit() is called after all operations have
// began executing (but does not guarantee that all operations have
// completed).
signaling_thread()->PostTask(
FROM_HERE,
base::BindOnce(&WebRtcMediaStreamTrackAdapterMapStressTest::
QuitRunLoopOnSignalingThread,
base::Unretained(this), base::Unretained(run_loop)));
}
}
void PostMainThreadLoop(base::RunLoop* run_loop) {
main_thread_->PostTask(
FROM_HERE,
base::BindOnce(
&WebRtcMediaStreamTrackAdapterMapStressTest::MainThreadLoop,
base::Unretained(this), base::Unretained(run_loop)));
}
void SignalingThreadLoop() {
std::vector<std::unique_ptr<WebRtcMediaStreamTrackAdapterMap::AdapterRef>>
track_refs;
for (size_t i = 0u; i < 5u; ++i) {
track_refs.push_back(map_->GetOrCreateRemoteTrackAdapter(
MockWebRtcAudioTrack::Create("remote_track_id")));
}
main_thread_->PostTask(
FROM_HERE,
base::BindOnce(&WebRtcMediaStreamTrackAdapterMapStressTest::
DestroyAdapterRefsOnMainThread,
base::Unretained(this), std::move(track_refs)));
}
void PostSignalingThreadLoop() {
signaling_thread()->PostTask(
FROM_HERE,
base::BindOnce(
&WebRtcMediaStreamTrackAdapterMapStressTest::SignalingThreadLoop,
base::Unretained(this)));
}
void DestroyAdapterRefsOnMainThread(
std::vector<std::unique_ptr<WebRtcMediaStreamTrackAdapterMap::AdapterRef>>
track_refs) {}
void QuitRunLoopOnSignalingThread(base::RunLoop* run_loop) {
run_loop->Quit();
}
private:
size_t remaining_iterations_;
};
TEST_F(WebRtcMediaStreamTrackAdapterMapStressTest, StressTest) {
const size_t kNumStressTestIterations = 1000u;
RunStressTest(kNumStressTestIterations);
}
} // namespace content } // namespace content
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment