Closed
Bug 806375
Opened 10 years ago
Closed 10 years ago
PeerConnection DataChannel demo doesn't work a second time, requires restart
Categories
(Core :: WebRTC, defect, P1)
Core
WebRTC
Tracking
()
VERIFIED
FIXED
mozilla20
People
(Reporter: ted, Assigned: jesup)
References
()
Details
(Whiteboard: [WebRTC], [blocking-webrtc+])
Attachments
(2 files, 4 obsolete files)
8.83 KB,
text/html
|
Details | |
29.24 KB,
patch
|
mcmanus
:
review+
|
Details | Diff | Splinter Review |
I'm using an updated copy of the local_data_chat.html demo. It works fine in my Mac nightly (19.0a1 (2012-10-29)), but if I reload the page and try again it doesn't work. Restarting the browser makes it work again.
Assignee | ||
Comment 1•10 years ago
|
||
Looks like a pseudo-port reuse issue, though we should be releasing the DataChannelConnection and dropping the mMasterSocket
Assignee: nobody → rjesup
Updated•10 years ago
|
Priority: -- → P1
Whiteboard: [WebRTC], [blocking-webrtc+]
Assignee | ||
Comment 2•10 years ago
|
||
WIP
Assignee | ||
Comment 3•10 years ago
|
||
rebased onto current tip
Assignee | ||
Updated•10 years ago
|
Attachment #691207 -
Attachment is obsolete: true
Assignee | ||
Comment 4•10 years ago
|
||
Assignee | ||
Comment 5•10 years ago
|
||
WIP 2 - fixed almost-every-time crash on shutdown of the connection (don't try to spawn an async runnable with a strong ref from within your destructor). Still crashes often in pipeline code, unrelated to datachannels
Assignee | ||
Updated•10 years ago
|
Attachment #691211 -
Attachment is obsolete: true
Assignee | ||
Comment 6•10 years ago
|
||
Working patch that lets me start up and shut down DataChannelConnections multiple times. Still get crashes on shutdown in the pipeline/Render code if I use video instead of audio, but that's a separate bug and doesn't happen all the time. Might need a little cleanup of logging/etc for checkin.
Assignee | ||
Updated•10 years ago
|
Attachment #691341 -
Attachment is obsolete: true
Assignee | ||
Comment 7•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #691831 -
Attachment is obsolete: true
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 691858 [details] [diff] [review] cleanup especially of channel close and connection shutdown Major cleanup of channel close (didn't really work) and connection shutdown (really didn't work). Also move connect to a more mozilla-ish paradigm (nsIThread/runnable instead of PR_Thread). SCTP debugs no longer require rebuilding to use (use datachannel:6). We could separate it into a separate log ID instead. I think there's still an issue in the sctp library with sctp releasing ports on socket close (it seems to be, but it still shows as inuse when we try to bind again), but with REUSE_PORT set, it's not an issue for now.
Attachment #691858 -
Flags: review?(mcmanus)
Assignee | ||
Comment 9•10 years ago
|
||
The port reuse issue appears to have been an artifact of it only shutting one of the two ports at the time; with the current patch I can disable REUSE_PORT. We may want to keep it for other reasons (in particular Offer forking/cloning).
Comment 10•10 years ago
|
||
Comment on attachment 691858 [details] [diff] [review] cleanup especially of channel close and connection shutdown Review of attachment 691858 [details] [diff] [review]: ----------------------------------------------------------------- this lgtm.. particularly the general necko and sctp bits - there are some internal webrtc bits that are opaque to me. The reuse port stuff should be fine for our application so I don't mind it being there.
Attachment #691858 -
Flags: review?(mcmanus) → review+
Assignee | ||
Comment 11•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/916f6915112d
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → mozilla20
Assignee | ||
Comment 12•10 years ago
|
||
Backout for linux bustage due to anonymous enums causing ambiguous function signatures https://hg.mozilla.org/integration/mozilla-inbound/rev/0685e3a994e0
Assignee | ||
Comment 13•10 years ago
|
||
Re-land after try push: https://hg.mozilla.org/integration/mozilla-inbound/rev/fe5784e9e50b
Comment 14•10 years ago
|
||
Randell, can this be tested via an unit test or do we need a mochitest for?
Status: NEW → ASSIGNED
Comment 15•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/fe5784e9e50b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 16•10 years ago
|
||
We were hitting this bug for our https://github.com/mozilla/socialapi-demo demo; we worked around it by randomizing the port numbers used for the data channel: https://github.com/mozilla/socialapi-demo/commit/d6ea884a1cc101b10414c7892721df6097a249f9 I've just verified that this bug is fixed by reverting locally our work-around. I can reproduce the bug with: Build identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:20.0) Gecko/20121212 Firefox/20.0 I cannot reproduce the bug any more with: Build identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:20.0) Gecko/20121218 Firefox/20.0
Comment 17•10 years ago
|
||
Verified in 2/4/2013 - I can use the data channel demo after a few reloads.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Updated•10 years ago
|
Flags: in-testsuite?
You need to log in
before you can comment on or make changes to this bug.
Description
•