New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 796704 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Clean up potential WMPI/ChunkDemuxer teardown race with WMPI::OnMemoryPressure

Project Member Reported by wolenetz@chromium.org, Dec 20 2017

Issue description

WMPI::OnMemoryPressure should post a task to a weak-ptr of itself on the media thread to notify ChunkDemuxer of the memory pressure change. Existing code looks like it allows for a race in WMPI::OnMemoryPressure + ~WMPI (and demuxer destruction) to occur before the task on the media thread in ChunkDemuxer is run.
 
Status: Started (was: Assigned)
An alternative fix for this (make ~WMPI's destruction of chunk_demuxer_ be done on a task chain that hops through media_task_runner_, ensuring that the OnMemoryPressure task doesn't race the destruction of the ChunkDemuxer) is in CQ now: https://chromium-review.googlesource.com/c/chromium/src/+/838701.
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/95af6365f9aedc89162d3f7efe4fb7453f242cb2

commit 95af6365f9aedc89162d3f7efe4fb7453f242cb2
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Thu Jan 04 22:44:13 2018

MSE: Destroy WMPI's ChunkDemuxer object using a background task

This change helps avoid renderer hang crashes on Windows by doing the
potentially costly destruction of a ChunkDemuxer object on a background
task.  The duration of that destruction, not including any initial
scheduling delay, is recorded in a new histogram:
"Media.MSE.DemuxerDestructionTime".

BUG= 774268 ,774288, 796704 

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I77dcb3ac1f7d5c5777f5302a7fc87199bf693d74
Reviewed-on: https://chromium-review.googlesource.com/838701
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527125}
[modify] https://crrev.com/95af6365f9aedc89162d3f7efe4fb7453f242cb2/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/95af6365f9aedc89162d3f7efe4fb7453f242cb2/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/95af6365f9aedc89162d3f7efe4fb7453f242cb2/media/filters/chunk_demuxer_unittest.cc
[modify] https://crrev.com/95af6365f9aedc89162d3f7efe4fb7453f242cb2/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Started)
#2 takes an approach that doesn't trigger this issue (per #1).
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 2 2018

Labels: merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/12d7ef018b9875e805960378e68048cb3e0b4dca

commit 12d7ef018b9875e805960378e68048cb3e0b4dca
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Fri Feb 02 01:43:09 2018

To M64: MSE: Destroy WMPI's ChunkDemuxer object using a background task

This change helps avoid renderer hang crashes on Windows by doing the
potentially costly destruction of a ChunkDemuxer object on a background
task.  The duration of that destruction, not including any initial
scheduling delay, is recorded in a new histogram:
"Media.MSE.DemuxerDestructionTime".

BUG= 774268 ,774288, 796704 
TBR=wolenetz@chromium.org, robliao@chromium.org, holte@chromium.org, dalecurtis@chromium.org

(cherry picked from commit 95af6365f9aedc89162d3f7efe4fb7453f242cb2)

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I77dcb3ac1f7d5c5777f5302a7fc87199bf693d74
Reviewed-on: https://chromium-review.googlesource.com/838701
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#527125}
Reviewed-on: https://chromium-review.googlesource.com/898077
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#635}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/12d7ef018b9875e805960378e68048cb3e0b4dca/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/12d7ef018b9875e805960378e68048cb3e0b4dca/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/12d7ef018b9875e805960378e68048cb3e0b4dca/media/filters/chunk_demuxer_unittest.cc
[modify] https://crrev.com/12d7ef018b9875e805960378e68048cb3e0b4dca/tools/metrics/histograms/histograms.xml

Sign in to add a comment