Closed Bug 1560611 Opened 5 months ago Closed 4 months ago

Crash in [@ java.lang.NullPointerException: at org.mozilla.gecko.media.CodecProxy.resetBuffers(CodecProxy.java)]

Categories

(Core :: Audio/Video: Playback, defect, P2, critical)

Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 69+ fixed
firefox68 --- wontfix
firefox69 --- fixed
firefox70 --- fixed

People

(Reporter: marcia, Assigned: jhlin)

References

(Regression)

Details

(Keywords: crash, regression, Whiteboard: [fennec68.1])

Crash Data

Attachments

(1 file)

This bug is for crash report bp-20cabe8e-2099-45eb-9a64-35fbd0190621.

Seen while looking at nightly: https://bit.ly/2XuohnF. Crash is new in 68 and started in 20190509033505.

Comments: trying to connect to talky.io

Possible regression ranged based on build id: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=65a693623cee0837b4ad0d23241c84cd3ea23e3a&tochange=34a824c75b7b5618a06ba8987c418d6363da5038

Java stack trace:

java.lang.NullPointerException
	at org.mozilla.gecko.media.CodecProxy.resetBuffers(CodecProxy.java:310)
	at org.mozilla.gecko.media.CodecProxy.flush(CodecProxy.java:296)

Discussed during triage - we need a priority and to figure out a possible regression range here. Thanks.

Flags: needinfo?(drno)

John, could you please have a look at what is going on here?

Flags: needinfo?(drno) → needinfo?(jolin)
Priority: -- → P2

The crashing code is from bug 1540036/D26188.

The elements in mInputBuffers were added in [1] and used immediately at the next line. If any of them were null, the exception should be raised there. Will review the code again to see what could go wrong.

[1] https://searchfox.org/mozilla-central/source/mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/CodecProxy.java#263

Assignee: nobody → jolin
Flags: needinfo?(jolin)

Should we consider backing out 1540036 from esr68?

Regressed by: 1540036

Found what went wrong: the NPE from fillInputBuffer() is caught in input() and translated to error callback[1], so [2] should be guarded with nullness check. I will upload the fix and request uplift after landing in central and beta asap.

[1] https://searchfox.org/mozilla-central/source/mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/CodecProxy.java#246
[2] https://searchfox.org/mozilla-central/source/mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/CodecProxy.java#266

Pushed by jolin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0c4414e7ab91
remember valid buffers only. r=jya
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70

Please nominate this for Beta and ESR68 approval when you get a chance.

Comment on attachment 9078271 [details]
Bug 1560611 - remember valid buffers only. r?jya

Beta/Release Uplift Approval Request

  • User impact if declined: Intermittent crashes when seeking video.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The change is trivial and simple.
  • String changes made/needed:

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Radom crashes are annoying.
  • User impact if declined: Intermittent crashes
  • Fix Landed on Version: 70
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The change is trivial and simple.
  • String or UUID changes made by this patch:
Flags: needinfo?(jolin)
Attachment #9078271 - Flags: approval-mozilla-esr68?
Attachment #9078271 - Flags: approval-mozilla-beta?

(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)

Please nominate this for Beta and ESR68 approval when you get a chance.

Done. Thanks a lot for the reminder!

Comment on attachment 9078271 [details]
Bug 1560611 - remember valid buffers only. r?jya

Fixes a Fennec crash. Approved for 68.1b3.

Attachment #9078271 - Flags: approval-mozilla-esr68?
Attachment #9078271 - Flags: approval-mozilla-esr68+
Attachment #9078271 - Flags: approval-mozilla-beta?
Attachment #9078271 - Flags: approval-mozilla-beta+
Whiteboard: [fennec68.1]
You need to log in before you can comment on or make changes to this bug.