cubeb_resampler_speex calls data callback while draining

RESOLVED FIXED in Firefox 58

Status

()

P1
critical
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

({crash, regression})

33 Branch
mozilla59
All
Windows
crash, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox57 wontfix, firefox58+ fixed, firefox59 fixed)

Details

(crash signature)

Attachments

(1 attachment)

(Assignee)

Description

a year ago
https://github.com/kinetiknz/cubeb/pull/398

+++ This bug was initially created as a clone of Bug #1418820 +++

Filing a separate bug from 1418820 because patches have already landed for
1418820.

This is a suspected common cause of crashes reported in bug 1418820 and possibly many other MSG crashes on NT and Android.  The bug was reproduced with a gtest in the pull request.
Comment hidden (mozreview-request)
(Assignee)

Comment 2

a year ago
mozreview-review
Comment on attachment 8941722 [details]
bug 1429666 cubeb_resampler_speex: don't call data callback while draining

https://reviewboard.mozilla.org/r/211942/#review217726

::: media/libcubeb/README_MOZILLA:5
(Diff revision 1)
> +and the following patches, which may be overwritten when
> +included upstream.
> +https://github.com/kinetiknz/cubeb/pull/398/commits/c8e66dee61a35e6a6d54e3630e1668bdbd6984b4
> +https://github.com/kinetiknz/cubeb/pull/398/commits/2ed979bc891cf1a7822799947a357d4d3b625964

This intentionally only applies the changesets in
https://github.com/kinetiknz/cubeb/pull/398 rather than pulling libcubeb from
upstream, because we'll want to uplift this change to other branches.

I'm requesting review now, before receiving upstream review, in case I can get
review by tomorrow, because I don't expect to be available for some time after
that.

Comment 3

a year ago
mozreview-review
Comment on attachment 8941722 [details]
bug 1429666 cubeb_resampler_speex: don't call data callback while draining

https://reviewboard.mozilla.org/r/211942/#review217766
Attachment #8941722 - Flags: review?(padenot) → review+
(Assignee)

Updated

a year ago
Keywords: leave-open

Comment 5

a year ago
Pushed by ktomlinson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1ad55edca07d
cubeb_resampler_speex: don't call data callback while draining r=padenot

Comment 6

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1ad55edca07d
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment on attachment 8941722 [details]
bug 1429666 cubeb_resampler_speex: don't call data callback while draining

Approval Request Comment
[Feature/Bug causing the regression]: bug 1008079
[User impact if declined]: Crashes on windows
[Is this code covered by automated tests?]: Yes, to be landed later (they are in https://github.com/kinetiknz/cubeb/pull/398, that we'll import in gecko soon).
[Has the fix been verified in Nightly?]: Yes, very clear (see https://github.com/mozilla/crashstop results)
[Needs manual test from QE? If yes, steps to reproduce]: No, this is covered by a unit test
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: This is basically just a big if() around the problematic code, to restore an invariant that was documented all along.
[String changes made/needed]: None.
Attachment #8941722 - Flags: approval-mozilla-beta?
Comment on attachment 8941722 [details]
bug 1429666 cubeb_resampler_speex: don't call data callback while draining

Hi Gerry, Philipp mentioned this would be a good candidate to include in the next RC build. Moving the nom from m-b to m-r for your review for next RC build.
Flags: needinfo?(gchang)
Attachment #8941722 - Flags: approval-mozilla-beta? → approval-mozilla-release?
status-firefox58: --- → affected
tracking-firefox58: --- → ?
Comment on attachment 8941722 [details]
bug 1429666 cubeb_resampler_speex: don't call data callback while draining

Fix a crash. Beta58+.
Flags: needinfo?(gchang)
Attachment #8941722 - Flags: approval-mozilla-release? → approval-mozilla-release+
tracking-firefox58: ? → +
status-firefox57: --- → wontfix
status-firefox-esr52: --- → wontfix
(Assignee)

Updated

a year ago
See Also: → bug 1414510
(Assignee)

Updated

a year ago
See Also: → bug 1415755
You need to log in before you can comment on or make changes to this bug.