Closed
Bug 822197
Opened 11 years ago
Closed 11 years ago
Assertion failure: cb_hdr and crash [@ fim_process_event/FIM_DEBUG]
Categories
(Core :: WebRTC: Signaling, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: posidron, Assigned: ehugg)
References
Details
(Keywords: assertion, crash, testcase, Whiteboard: [WebRTC], [blocking-webrtc-], [qa-])
Crash Data
Attachments
(4 files, 1 obsolete file)
6.51 KB,
text/plain
|
Details | |
8.02 KB,
text/html
|
Details | |
4.44 KB,
patch
|
jesup
:
review+
|
Details | Diff | Splinter Review |
1.40 KB,
patch
|
ehugg
:
review+
|
Details | Diff | Splinter Review |
./media/webrtc/signaling/src/sipcc/core/gsm/fim.c:538 [...] if (call_chn == NULL) { FIM_DEBUG(get_debug_string(GSM_DBG1), "FIM", call_id, fname, "no call_chn"); [...] @rforbes SEED: 1355734171.18 Fault was detected on test 111 Tested with m-i changeset: 116243:9d6e95e77855
Updated•11 years ago
|
Assignee: nobody → ethanhugg
Priority: -- → P3
Whiteboard: [WebRTC], [blocking-webrtc-]
Assignee | ||
Comment 1•11 years ago
|
||
Unfortunately this callstack is not informative enough for me to figure this out. If you get another crash in this area, please post it. Thanks.
Reporter | ||
Comment 2•11 years ago
|
||
Just a note that this crash still pops up relatively often with m-c changeset: 118049:928550157e6e
Reporter | ||
Comment 3•11 years ago
|
||
I am seeing this assertion failure before the crash: Assertion failure: cb_hdr, at /Users/cdiehl/Code/Mozilla/mc-inbound-asan/media/webrtc/signaling/src/sipcc/core/gsm/fim.c:604
Updated•11 years ago
|
Keywords: assertion
Summary: WebRTC crash [@fim_process_event/FIM_DEBUG] → Assertion failure: cb_hdr and crash [@ fim_process_event/FIM_DEBUG]
Assignee | ||
Comment 4•11 years ago
|
||
I just got this to happen under the debugger. It appears to happen when we go over the MAX_CONFIG_LINES limit in SIPCC. If I hit reload quickly perhaps I'm creating PCs faster than they are being destructed. Not sure what to do about it yet, whether we even need a maximum number here, whether it could be arbitrarily large, or whether we can make a saner error case when it's hit.
Reporter | ||
Comment 5•11 years ago
|
||
(In reply to Ethan Hugg [:ehugg] from comment #4) > I just got this to happen under the debugger. If I hit reload quickly > perhaps I'm creating PCs faster than they are being destructed. That's great. That practice covers with my fuzzing procedures. Since I added a delay of 3 seconds between each test I am not seeing this crash anymore.
Assignee | ||
Comment 6•11 years ago
|
||
Low-brow testcase that creates and holds many PeerConnection objects and hits this assert every time for me.
Updated•11 years ago
|
Flags: in-testsuite?
Assignee | ||
Comment 7•11 years ago
|
||
Assignee | ||
Comment 8•11 years ago
|
||
Comment on attachment 702490 [details] [diff] [review] Handle creation of more PeerConnections than MAX_LINES Review of attachment 702490 [details] [diff] [review]: ----------------------------------------------------------------- In this case we are running out of a resource known as the feature control block which are allocated in a fixed array in fsm.c. This is the simple fix where we handle this with an error instead of a crash. Also note that after making 45 PeerConnections you will see this JavaScript error: file:///home/ehugg/mozilla/mozilla-central/obj-ff-dbg/dist/bin/components/PeerConnection.js, line 278: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [IPeerConnection.initialize] Which is from PeerConnectionMedia::Init/NrIceCtx::StartGathering which is has this maximum limit. I did not try to fix this since it is handled with a proper error and doesn't crash, but it appears 45 is our limit for making PCs. I have also considered changing the fcbs to be either a dynamically expandable array or a linked list so that they would only run out with memory. I feel that needs its own patch, but let me know if you think that would be a priority.
Attachment #702490 -
Flags: review?(rjesup)
Comment 9•11 years ago
|
||
Comment on attachment 702490 [details] [diff] [review] Handle creation of more PeerConnections than MAX_LINES Review of attachment 702490 [details] [diff] [review]: ----------------------------------------------------------------- We will likely need to address the number of slots as well, in a different bug
Attachment #702490 -
Flags: review?(rjesup) → review+
Assignee | ||
Comment 10•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/096ee26f5a0d
Comment 11•11 years ago
|
||
(In reply to Ethan Hugg [:ehugg] from comment #8) > Also note that after making 45 PeerConnections you will see this > > JavaScript error: > file:///home/ehugg/mozilla/mozilla-central/obj-ff-dbg/dist/bin/components/ > PeerConnection.js, line 278: NS_ERROR_FAILURE: Component returned failure > code: 0x80004005 (NS_ERROR_FAILURE) [IPeerConnection.initialize] Keep in mind that this is bug 824919 and will hopefully be fixed soon.
Comment 12•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/096ee26f5a0d
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Updated•11 years ago
|
Whiteboard: [WebRTC], [blocking-webrtc-] → [WebRTC], [blocking-webrtc-], [qa-]
Comment 13•11 years ago
|
||
Comment 14•11 years ago
|
||
Comment on attachment 706855 [details] [diff] [review] Crashtest Crashtest for this bug. I've taken Ethan's test case, cleaned some of the code up, and made it a crash test.
Attachment #706855 -
Flags: review?(ethanhugg)
Comment 15•11 years ago
|
||
Comment on attachment 706855 [details] [diff] [review] Crashtest >+ var Logger = { >+ console: function(msg) { >+ if (window.dump) { >+ window.dump(msg); >+ } else if (window.console && window.console.log) { >+ window.console.log(msg); >+ } >+ } >+ } [..] >+ >+ function onFailure(code) { >+ Logger.console("\n########## Failure: " + code + "\n"); >+ } I do not think that logging is necessary here to reproduce the crash. It has only put in to the manual testcase to see what's going on.
Attachment #706855 -
Flags: feedback-
Comment 16•11 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #15) > Comment on attachment 706855 [details] [diff] [review] > Crashtest > > >+ var Logger = { > >+ console: function(msg) { > >+ if (window.dump) { > >+ window.dump(msg); > >+ } else if (window.console && window.console.log) { > >+ window.console.log(msg); > >+ } > >+ } > >+ } > [..] > >+ > >+ function onFailure(code) { > >+ Logger.console("\n########## Failure: " + code + "\n"); > >+ } > > I do not think that logging is necessary here to reproduce the crash. It has > only put in to the manual testcase to see what's going on. I'm not sure that's right. Take a look at the patch attached here. I'll wait for Ethan's feedback. Clearing feedback- as there isn't a clear indication is correct or not.
Updated•11 years ago
|
Attachment #706855 -
Flags: feedback-
Assignee | ||
Comment 17•11 years ago
|
||
Comment on attachment 706855 [details] [diff] [review] Crashtest Review of attachment 706855 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me, just a couple nits. ::: dom/media/tests/crashtests/822197.html @@ +4,5 @@ > +https://bugzilla.mozilla.org/show_bug.cgi?id=822197 > +--> > +<head> > + <meta charset="utf-8"> > + <title>Bug 822917 - Many PeerConnections with CreateOffer</title> The bug number is 822197 - typo. @@ +22,5 @@ > + function onCreateOfferSuccess(offer) { > + } > + > + function onFailure(code) { > + Logger.console("\n########## Failure: " + code + "\n"); As Henrik mentions, logging is only applicable to a manual run. The loop below should create and hold enough PCs to test the crash.
Attachment #706855 -
Flags: review?(ethanhugg) → review+
Updated•11 years ago
|
Attachment #706855 -
Attachment is obsolete: true
Comment 18•11 years ago
|
||
Comment 19•11 years ago
|
||
Comment on attachment 707185 [details] [diff] [review] Crashtest v2 Cleaned up per review comments.
Attachment #707185 -
Flags: review?(ethanhugg)
Assignee | ||
Updated•11 years ago
|
Attachment #707185 -
Flags: review?(ethanhugg) → review+
Updated•11 years ago
|
Keywords: checkin-needed
Comment 20•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/60044584ed1e
Flags: in-testsuite? → in-testsuite+
Keywords: checkin-needed
Updated•11 years ago
|
Attachment #706855 -
Flags: feedback-
You need to log in
before you can comment on or make changes to this bug.
Description
•