MediaStreamSource: verify unlocked state when finalizing.

MediaStreamSource: verify unlocked state when finalizing.

To diagnose an audio thread crash condition, verify that the lock
over audio consumers that MediaStreamSource keeps, isn't held when it is
being finalized. If it is, then the audio thread is active using the
MediaStreamSource object..which is not a well-formed state to be in.

R=
BUG=682945
Review-Url: https://codereview.chromium.org/2741663004
Cr-Commit-Position: refs/heads/master@{#456029}
Committed: https://chromium.googlesource.com/chromium/src/+/c662576c8bb7cecef0dd9a699112fa5cc4b6ab79

[sof, 2017-03-09 21:59:54]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-09 22:00:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-10 00:26:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-10 00:26:47]

Dry run: This issue passed the CQ dry run.

[sof, 2017-03-10 08:30:39]

Description was changed from

==========
MediaStreamSource: verify unlocked state when finalizing.

To diagnose an audio thread crash condition, verify that the lock
over audio consumers that MediaStreamSource, isn't held when it is
being finalized. If it is, then the audio thread is active using the
object..which it cannot be.

R=
BUG=682945
==========

to

==========
MediaStreamSource: verify unlocked state when finalizing.

To diagnose an audio thread crash condition, verify that the lock
over audio consumers that MediaStreamSource heeps, isn't held when it is
being finalized. If it is, then the audio thread is active using the
MediaStreamSource object..which is not a well-formed state to be in.

R=
BUG=682945
==========

[sof, 2017-03-10 08:50:42]

sigbjornf@opera.com changed reviewers:
+ guidou@chromium.org, haraken@chromium.org, hongchan@chromium.org,
rtoy@chromium.org

[sof, 2017-03-10 08:50:43]

please take a look.

The underlying theory here is that MediaStreamSource::consumeAudio() is a longer
running operation for the audio thread in some cases, which can cause the main
thread to GC the nodes and objects the audio thread is accessing while it is
active.

The CHECK() added here aims to verify the potential validity of that theory. If
it is obviously false, then it has no merit to land this CL. iow, please say if
this is a tangent leading nowhere interesting.

[haraken, 2017-03-10 08:56:42]

LGTM

[sof, 2017-03-10 08:57:44]

Description was changed from

==========
MediaStreamSource: verify unlocked state when finalizing.

To diagnose an audio thread crash condition, verify that the lock
over audio consumers that MediaStreamSource heeps, isn't held when it is
being finalized. If it is, then the audio thread is active using the
MediaStreamSource object..which is not a well-formed state to be in.

R=
BUG=682945
==========

to

==========
MediaStreamSource: verify unlocked state when finalizing.

To diagnose an audio thread crash condition, verify that the lock
over audio consumers that MediaStreamSource keeps, isn't held when it is
being finalized. If it is, then the audio thread is active using the
MediaStreamSource object..which is not a well-formed state to be in.

R=
BUG=682945
==========

[sof, 2017-03-10 09:18:34]

thanks, let's try.

[sof, 2017-03-10 09:18:40]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2017-03-10 09:18:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-10 09:22:43]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1489137520006920, "parent_rev":
"afaa76ff45edbba9b6390ba1e6a792643c085093", "commit_rev":
"c662576c8bb7cecef0dd9a699112fa5cc4b6ab79"}

[commit-bot: I haz the power, 2017-03-10 09:23:20]

Description was changed from

==========
MediaStreamSource: verify unlocked state when finalizing.

To diagnose an audio thread crash condition, verify that the lock
over audio consumers that MediaStreamSource keeps, isn't held when it is
being finalized. If it is, then the audio thread is active using the
MediaStreamSource object..which is not a well-formed state to be in.

R=
BUG=682945
==========

to

==========
MediaStreamSource: verify unlocked state when finalizing.

To diagnose an audio thread crash condition, verify that the lock
over audio consumers that MediaStreamSource keeps, isn't held when it is
being finalized. If it is, then the audio thread is active using the
MediaStreamSource object..which is not a well-formed state to be in.

R=
BUG=682945

Review-Url: https://codereview.chromium.org/2741663004
Cr-Commit-Position: refs/heads/master@{#456029}
Committed:
https://chromium.googlesource.com/chromium/src/+/c662576c8bb7cecef0dd9a699112...
==========

[commit-bot: I haz the power, 2017-03-10 09:23:21]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/c662576c8bb7cecef0dd9a699112...

[hongchan, 2017-03-10 15:49:35]

On 2017/03/10 09:23:21, commit-bot: I haz the power wrote:
> Committed patchset #1 (id:1) as
>
https://chromium.googlesource.com/chromium/src/+/c662576c8bb7cecef0dd9a699112...

sof@ Thanks for working on this!

[sof, 2017-03-10 18:30:20]

On 2017/03/10 15:49:35, hongchan wrote:
> On 2017/03/10 09:23:21, commit-bot: I haz the power wrote:
> > Committed patchset #1 (id:1) as
> >
>
https://chromium.googlesource.com/chromium/src/+/c662576c8bb7cecef0dd9a699112...
> 
> sof@ Thanks for working on this!

I'll watch for reports of ~MediaStreamSource Canary crashes over the weekend;
fingers crossed..

[sof, 2017-03-22 09:59:37]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2765073004/ by sigbjornf@opera.com.

The reason for reverting is: Diagnosis completed, reverting (cf.
https://bugs.chromium.org/p/chromium/issues/detail?id=682945#c48 ).