1506884 - Audit and document member access from threads in AudioConduit

Status: RESOLVED FIXED

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

Description

[Dan Minor [:dminor]]

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.

Comment 1

[Dan Minor [:dminor]]

I think this will just end up bitrotting bug 1507861 unnecessarily.

Comment 2

[Dan Minor [:dminor]]

Attachment: Bug 1506884 - Document thread usage of mRtpPacketQueue in VideoConduit; r=padenot!

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

Comment 3

[Dan Minor [:dminor]]

Attachment: Bug 1506884 - Use DeleteStreams in AudioConduit; r=padenot!

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

Comment 4

[Dan Minor [:dminor]]

Attachment: Bug 1506884 - Document member thread access in AudioConduit; r=padenot!

This also removes some unused member variables.

Depends on D25055

Comment 5

[Dan Minor [:dminor]]

Attachment: Bug 1506884 - Remove capture_delay pref; r=padenot!

This is no longer used.

Depends on D25056

Comment 6

[Pulsebot]

Pushed by [e-mail]:
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

Comment 7

[Noemi Erli[:noemi_erli]]

Backed out 4 changesets (Bug 1506884) for mochitest failures CLOSED TREE

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=5840b7d8521a6c60a9dcee429f6682436101f27f&selectedJob=236439843

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=236442391&repo=autoland&lineNumber=35953

Backout: https://hg.mozilla.org/integration/autoland/rev/a0084ecbf6719531544190109d9a59ffa6747fe2

Comment 8

[Dan Minor [:dminor]]

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.

Comment 9

[Dan Minor [: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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
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

Comment 11

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/a8af6b72513d
https://hg.mozilla.org/mozilla-central/rev/938699e324d1
https://hg.mozilla.org/mozilla-central/rev/6de9b9ab5d22