1345342 - Crash in mozilla::H264Converter::Decode: MOZ_RELEASE_ASSERT(!mDecodePromiseRequest.Exists() && !mInitPromiseRequest.Exists())

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[Kan-Ru Chen [:kanru] (UTC+9)]

This bug was filed from the Socorro interface and is 
report bp-af61d399-8882-4e1c-b721-ddba12170217.
=============================================================

It looks like it's only on FennecAndroid. The first crash appears in build 20170216110238. Bug 1336431 looks suspicious to me.

Comment 1

[Blake Wu [:bwu][:blakewu]]

Put this on John's plate. :-)

Comment 2

[Marcia Knous [:marcia]]

This is the top crash on Fennec Aurora in the last 7 days: https://crash-stats.mozilla.com/topcrashers/?product=FennecAndroid&version=54.0a2&days=7.

Can we get a follow up on Comment 1?  Thanks!

Comment 3

[John Lin [:jhlin][:jolin]]

Oops! I think this can be fixed by bug 1345599, which I forgot to uplift to aurora. Sorry about that.

Comment 4

[Jean-Yves Avenard [:jya]]

Are you sure? the crash is due to an assert where somehow H264Converter::Decoder gets called prior the previous decode promise (or init promise) having completed.

That can only be a bug in H264Converter I think...

Comment 5

[John Lin [:jhlin][:jolin]]

(In reply to Jean-Yves Avenard [:jya] from comment #4)
> Are you sure? the crash is due to an assert where somehow
> H264Converter::Decoder gets called prior the previous decode promise (or
> init promise) having completed.
> 
> That can only be a bug in H264Converter I think...

 Those 2 promise requests asserted are only tracked in DecodeFirstSample() and CreateDecoderAndInit(), respectively.
 It seems the crashes started to happen after bug 1336431, so it's likely that the modifications there causes this problem.
 Only DecodeFirstSample() is mentioned in one of the patches and that was reverted in bug 1345599, so I figured it can fix the crash.

Comment 6

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Happening in 54 but not seen in 55.

Comment 7

[[:philipp]]

this crash signature is starkly spiking up on desktop nightly after build 20170602030204 to crashes with over 100 installations per day.
those reports, like bp-964edd66-15a7-471d-87b6-2031c0170604, all have "MOZ_RELEASE_ASSERT(!mDecodePromiseRequest.Exists() && !mInitPromiseRequest.Exists()) (Can't request a new decode until previous one completed)" added in bug 1319987

this would be the changelog to the day before: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bdb2387396b4a74dfefb7c983733eed3625e906a&tochange=aeb3d0ca558f034cbef1c5a68bd07dd738611494

do you think it could be related to the patch from bug 1313398 landing in that timeframe?

Comment 8

[Jean-Yves Avenard [:jya]]

this is very late in the 54 cycle, but I'm hoping that can still be uplifted

Comment 9

[Jean-Yves Avenard [:jya]]

(In reply to [:philipp] from comment #7)
> do you think it could be related to the patch from bug 1313398 landing in
> that timeframe?

not directly, however it can increase the circumstances under which we re-initialise the decoder..

Comment 10

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1345342: Cancel pending requests.

If the pending first frame decoding was cancelled, the next call to decode could lead to a crash.

Review commit: https://reviewboard.mozilla.org/r/145616/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/145616/

Comment 11

[Nicholas Nethercote [inactive]]

This is the #2 Windows topcrash in Nightly 20170602030204. It doesn't show up at all in Nightly 20170602100159. So the likely regression range is here:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bdb2387396b4a74dfefb7c983733eed3625e906a&tochange=aeb3d0ca558f034cbef1c5a68bd07dd738611494

Bug 1313398 (jya) and bug 1368837 (gsquelart) are the two bugs in that range that look most relevant, though I may be overlooking something.

Comment 12

[Jean-Yves Avenard [:jya]]

I've submitted a fix.. not sure that there's more I can do at this stage!

Comment 13

[Gerry Chang [:gchang]]

From the crash report, the volume of crashes in 54 is very low. Given we are about to go to RC week, I prefer to let it ride the train and won't fix in 54.

Comment 14

[Jean-Yves Avenard [:jya]]

Comment on attachment 8874191 [details]
Bug 1345342: Cancel pending requests.

https://reviewboard.mozilla.org/r/145616/#review149616

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ff724e355b2d
Cancel pending requests. r=jya

Comment 16

[Cameron McCormack (:heycam)]

Backed out in: https://hg.mozilla.org/integration/autoland/rev/50075548c1ef

Comment 17

[Jean-Yves Avenard [:jya]]

Comment on attachment 8874191 [details]
Bug 1345342: Cancel pending requests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/145616/diff/1-2/

Comment 18

[Jean-Yves Avenard [:jya]]

Comment on attachment 8874191 [details]
Bug 1345342: Cancel pending requests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/145616/diff/2-3/

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e78bd3e1e16
Cancel pending requests. r=jya

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8e78bd3e1e16

Comment 21

[[:philipp]]

the signature is still present after this patch has landed: https://crash-stats.mozilla.com/signature/?build_id=%3E%3D20170606030207&version=55.0a1&signature=mozilla%3A%3AH264Converter%3A%3ADecode#reports

Comment 22

[Jean-Yves Avenard [:jya]]

will be handled in bug 1370805.

Comment 23

[Julian Seward [:jseward]]

This is still present in the nightly of 20170608030205, and is the 
#2 windows topcrash.

Comment 24

[Jean-Yves Avenard [:jya]]

The last report is 20170608030205, which doesn't include the fix.