WebRTC crash [@gsmsdp_negotiate_media_lines]

RESOLVED WONTFIX

Status

()

Core
WebRTC: Signaling
--
critical
RESOLVED WONTFIX
5 years ago
3 years ago

People

(Reporter: posidron, Assigned: abr)

Tracking

(Blocks: 1 bug, {crash, testcase})

Trunk
x86_64
Mac OS X
crash, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox24 fixed, firefox25 fixed)

Details

(Whiteboard: [leave-open], crash signature)

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 774551 [details]
testcase

Tested with hg.mozilla.org/integration/mozilla-inbound/rev/60865db5f5dc
(Reporter)

Comment 1

5 years ago
Created attachment 774552 [details]
callstack
(Reporter)

Comment 2

5 years ago
Created attachment 774553 [details]
console.txt
(Assignee)

Updated

5 years ago
Assignee: nobody → adam
(Assignee)

Comment 3

5 years ago
Created attachment 774570 [details] [diff] [review]
Check that media section is found before adding rtcp-fb attributes
(Assignee)

Comment 4

5 years ago
Comment on attachment 774570 [details] [diff] [review]
Check that media section is found before adding rtcp-fb attributes

I beleive the trigger here is the recvonly on the video line, where PC2 doesn't generate a corresponding media section. In any case, the trigger here is that we can reach this point in the code with media == 0. This check catches that situation.
Attachment #774570 - Flags: review?(ekr)

Comment 5

5 years ago
Hmm... I note that the error is in setremote. I wonder if perhaps you shouldn't be adding the
rtcp-fb in this case at all. Won't you get another crack at it in createanswer?

Comment 6

5 years ago
There's a space after @ in the signature so that it shows up in crash stats.
Crash Signature: [@ gsmsdp_negotiate_media_lines]
(Assignee)

Comment 7

5 years ago
Created attachment 774722 [details] [diff] [review]
Check that media section is found before adding rtcp-fb attributes
(Assignee)

Updated

5 years ago
Attachment #774570 - Attachment is obsolete: true
Attachment #774570 - Flags: review?(ekr)
(Assignee)

Comment 8

5 years ago
Comment on attachment 774722 [details] [diff] [review]
Check that media section is found before adding rtcp-fb attributes

You're right that this only makes sense when we're crafting an answer. I've adjusted the conditional accordingly. I'd still like to keep the null check for media in place; I can't rule out the possibility that we might get here with a null "media" even in the create_answer case.
Attachment #774722 - Flags: review?(ekr)
(Assignee)

Comment 9

5 years ago
r=ekr on the original patch via IRC.

https://hg.mozilla.org/integration/mozilla-inbound/rev/75d923fcb71c

Marking [leave-open] because I need to add some mochi tests here.
Whiteboard: [leave-open]
(Assignee)

Comment 11

5 years ago
Comment on attachment 774722 [details] [diff] [review]
Check that media section is found before adding rtcp-fb attributes

Setting r+ from IRC conversation
Attachment #774722 - Flags: review?(ekr) → review+
(Assignee)

Comment 12

5 years ago
Comment on attachment 774570 [details] [diff] [review]
Check that media section is found before adding rtcp-fb attributes

Okay, this is the one that ekr r+'d (and the one that we landed). Marking as such and requesting aurora uplift:

[Approval Request Comment]

Bug caused by (feature/regressing bug #): Bug 880067

User impact if declined: Browser crashiness

Testing completed (on m-c, etc.): Verified not to crash on attached testcase. Landed on m-i on 7/12, with no attributable regressions since then.

Risk to taking this patch (and alternatives if risky): This is the lowest risk patch you'll ever see -- all it does is check a pointer for NULL before dereferencing it.

String or IDL/UUID changes made by this patch: None
Attachment #774570 - Attachment is obsolete: false
Attachment #774570 - Flags: review+
Attachment #774570 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 13

5 years ago
Comment on attachment 774722 [details] [diff] [review]
Check that media section is found before adding rtcp-fb attributes

Obsoleting unapproved patch
Attachment #774722 - Attachment is obsolete: true
Attachment #774722 - Flags: review+

Updated

5 years ago
Attachment #774570 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-firefox24: --- → fixed
status-firefox25: --- → fixed
Flags: in-testsuite?
(Assignee)

Updated

4 years ago
Depends on: 1080765

Comment 15

3 years ago
sdparta removed this function
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.