Commit 1973f0b6 authored by dalecurtis's avatar dalecurtis Committed by Commit bot

Avoid FFmpeg destroying active resources during demuxer stop.

The root cause is a combination of BufferedDataSource::Stop()
requiring a lock and BlockingURLProtocol::Read() returning
control to FFmpeg which may destory resources in use by an
ongoing BufferedDataSource::ReadCallback() call.

The lock in BDS::Stop() shared by BDS::ReadCallback() prevents
the stop signal from taking affect until after the read finishes,
however calling BUP::Abort() will immediately returns control to
FFmpeg which may destroy the resources in use by BDS::ReadCallback().

Sadly, given the racy nature and multi-class dependencies I've been
unable to craft a unit test for this, but I've manually verified this
fix works by arbitrarily delaying BDS::Stop() under an ASAN build.

BUG=411318
TEST=none

Review URL: https://codereview.chromium.org/544843005

Cr-Commit-Position: refs/heads/master@{#293995}
parent 87305fa7
......@@ -557,8 +557,12 @@ FFmpegDemuxer::~FFmpegDemuxer() {}
void FFmpegDemuxer::Stop() {
DCHECK(task_runner_->BelongsToCurrentThread());
url_protocol_->Abort();
// The order of Stop() and Abort() is important here. If Abort() is called
// first, control may pass into FFmpeg where it can destruct buffers that are
// in the process of being fulfilled by the DataSource.
data_source_->Stop();
url_protocol_->Abort();
// This will block until all tasks complete. Note that after this returns it's
// possible for reply tasks (e.g., OnReadFrameDone()) to be queued on this
......
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