Closed Bug 1506884 Opened 10 months ago Closed 6 months ago

Audit and document member access from threads in AudioConduit

Categories

(Core :: WebRTC: Audio/Video, enhancement, P2)

65 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- fixed

People

(Reporter: dminor, Assigned: dminor)

Details

Attachments

(4 files)

Bug 1376873 added thread assertions to AudioConduit. We should follow up and audit and document which members are accessed from which threads, adding locks if required.
Assignee: nobody → dminor
I think this will just end up bitrotting bug 1507861 unnecessarily.
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1507861
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

This was added during the branch 64 update, but was not documented at that time.

This makes the shutdown behaviour of AudioConduit consistent with
VideoConduit which should make the conduit code easier to read and
reason about.

Depends on D25054

This also removes some unused member variables.

Depends on D25055

This is no longer used.

Depends on D25056

Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1584d9804aa1
Document thread usage of mRtpPacketQueue in VideoConduit; r=padenot
https://hg.mozilla.org/integration/autoland/rev/5837db3740b5
Use DeleteStreams in AudioConduit; r=padenot
https://hg.mozilla.org/integration/autoland/rev/0182d6543001
Document member thread access in AudioConduit; r=padenot
https://hg.mozilla.org/integration/autoland/rev/c1e32495cfa2
Remove capture_delay pref; r=padenot

I think the problematic commit is likely https://hg.mozilla.org/integration/autoland/rev/5837db3740b5. I think it is intermittent, I saw it on an earlier try job, but not the one just prior to landing. I don't think the DeleteStreams cleanup in 5837db3740b5 is worth risking introducing an intermittent shutdown crash, so I'm retrying with the other three commits. If that looks good, I'll land those three, and we can worry about DeleteStreams another time.

Flags: needinfo?(dminor)

Try job without 5837db3740b5: https://treeherder.mozilla.org/#/jobs?repo=try&revision=903fdd84ead7ad60b2faa59275bdd3e13fb6fcc7

I was concerned about the number of mda2 failures on linux64-asan, but it appears that is just a frequent intermittent: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c69975daf2cd53028561f440f52721e3b1a00d21

Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8af6b72513d
Document thread usage of mRtpPacketQueue in VideoConduit; r=padenot
https://hg.mozilla.org/integration/mozilla-inbound/rev/938699e324d1
Document member thread access in AudioConduit; r=padenot
https://hg.mozilla.org/integration/mozilla-inbound/rev/6de9b9ab5d22
Remove capture_delay pref; r=padenot
Status: REOPENED → RESOLVED
Closed: 10 months ago6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.