Closed Bug 806375 Opened 10 years ago Closed 10 years ago

PeerConnection DataChannel demo doesn't work a second time, requires restart


(Core :: WebRTC, defect, P1)






(Reporter: ted, Assigned: jesup)




(Whiteboard: [WebRTC], [blocking-webrtc+])


(2 files, 4 obsolete files)

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.
Looks like a pseudo-port reuse issue, though we should be releasing the DataChannelConnection and dropping the mMasterSocket
Assignee: nobody → rjesup
Depends on: 807103
Priority: -- → P1
Whiteboard: [WebRTC], [blocking-webrtc+]
rebased onto current tip
Attachment #691207 - Attachment is obsolete: true
Attached file Updated testcase
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
Attachment #691211 - Attachment is obsolete: true
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.
Attachment #691341 - Attachment is obsolete: true
Attachment #691831 - Attachment is obsolete: true
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)
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 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+
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → mozilla20
Backout for linux bustage due to anonymous enums causing ambiguous function signatures
Randell, can this be tested via an unit test or do we need a mochitest for?
Closed: 10 years ago
Resolution: --- → FIXED
Keywords: verifyme
We were hitting this bug for our demo; we worked around it by randomizing the port numbers used for the data channel:

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
Verified in 2/4/2013 - I can use the data channel demo after a few reloads.
Keywords: verifyme
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.