Closed Bug 950990 Opened 6 years ago Closed 6 years ago

ICE connection state switches from failed to open

Categories

(Core :: WebRTC: Networking, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: drno, Assigned: bwc)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

With a firewall blocking packets on loopback the ICE connection state first properly goes to failed, but then after some time suddenly switches to open/connected.
Note: this is reproducible by turning on the Mac OSX firewall and adding the ICE mochitest patch from bug 948249 to the mochitests.

Unfortunately I have failed to collect debug output from NrIcer so far. Here is the regular output:

7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): gathering state 0->1
7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): trickling candidate candidate:0 1 UDP 2130379007 10.250.7.48 58316 typ host
7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): trickling candidate candidate:0 2 UDP 2130379006 10.250.7.48 57997 typ host
7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): trickling candidate candidate:0 1 UDP 2130379007 10.250.7.48 54814 typ host
7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): trickling candidate candidate:0 2 UDP 2130379006 10.250.7.48 61316 typ host
7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): trickling candidate candidate:0 1 UDP 2130379007 10.250.7.48 49559 typ host
7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): trickling candidate candidate:0 2 UDP 2130379006 10.250.7.48 50494 typ host
7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): trickling candidate candidate:1 1 UDP 1694236671 63.245.220.240 20341 typ srflx raddr 10.250.7.48 rport 58316
7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): trickling candidate candidate:1 2 UDP 1694236670 63.245.220.240 12406 typ srflx raddr 10.250.7.48 rport 57997
7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): trickling candidate candidate:1 1 UDP 1694236671 63.245.220.240 16167 typ srflx raddr 10.250.7.48 rport 54814
7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): trickling candidate candidate:1 2 UDP 1694236670 63.245.220.240 16116 typ srflx raddr 10.250.7.48 rport 61316
7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): trickling candidate candidate:1 1 UDP 1694236671 63.245.220.240 5680 typ srflx raddr 10.250.7.48 rport 49559
7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): trickling candidate candidate:1 2 UDP 1694236670 63.245.220.240 16265 typ srflx raddr 10.250.7.48 rport 50494
7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): gathering state 1->2
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): gathering state 0->1
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): trickling candidate candidate:0 1 UDP 2130379007 10.250.7.48 59178 typ host
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): trickling candidate candidate:0 2 UDP 2130379006 10.250.7.48 56842 typ host
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): trickling candidate candidate:0 1 UDP 2130379007 10.250.7.48 57509 typ host
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): trickling candidate candidate:0 2 UDP 2130379006 10.250.7.48 56001 typ host
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): trickling candidate candidate:0 1 UDP 2130379007 10.250.7.48 58759 typ host
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): trickling candidate candidate:0 2 UDP 2130379006 10.250.7.48 58936 typ host
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): trickling candidate candidate:1 2 UDP 1694236670 63.245.220.240 14123 typ srflx raddr 10.250.7.48 rport 56842
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): trickling candidate candidate:1 1 UDP 1694236671 63.245.220.240 17213 typ srflx raddr 10.250.7.48 rport 59178
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): trickling candidate candidate:1 1 UDP 1694236671 63.245.220.240 14842 typ srflx raddr 10.250.7.48 rport 57509
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): trickling candidate candidate:1 2 UDP 1694236670 63.245.220.240 8144 typ srflx raddr 10.250.7.48 rport 56001
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): trickling candidate candidate:1 1 UDP 1694236671 63.245.220.240 9110 typ srflx raddr 10.250.7.48 rport 58759
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): trickling candidate candidate:1 2 UDP 1694236670 63.245.220.240 8130 typ srflx raddr 10.250.7.48 rport 58936
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): gathering state 1->2
7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): state 0->1
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): state 0->1
7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): state 1->3
7131136[100330c50]: NrIceCtx(PC:0d1a46da2ede54e3): state 3->2
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): state 1->3
7131136[100330c50]: NrIceCtx(PC:c125bea29a1b8109): state 3->2
Blocks: 948249
Note: if my ICE test had a timeout of about 1 minute the test failed before the ICE Connection State would switch to "connected". After increasing the timeout to 2.5 minutes I get the above behavior.
Assignee: nobody → docfaraday
Status: NEW → ASSIGNED
Give this patch a shot.
Flags: needinfo?(drno)
Just verified locally with my additional ICE mochitest (which are not committed yet) that Byrons patch fixes the issue.
Flags: needinfo?(drno)
Comment on attachment 8348956 [details] [diff] [review]
NrIceCtx::ice_completed is fired even on failed contexts, so do not mark such contexts as completed.

Review of attachment 8348956 [details] [diff] [review]:
-----------------------------------------------------------------

Should be a trivial change.
Attachment #8348956 - Flags: review?(adam)
Comment on attachment 8348956 [details] [diff] [review]
NrIceCtx::ice_completed is fired even on failed contexts, so do not mark such contexts as completed.

Review of attachment 8348956 [details] [diff] [review]:
-----------------------------------------------------------------

Looks correct to me.
Attachment #8348956 - Flags: review?(adam) → review+
Attachment #8348956 - Flags: checkin?(adam)
Comment on attachment 8348956 [details] [diff] [review]
NrIceCtx::ice_completed is fired even on failed contexts, so do not mark such contexts as completed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1470c1a77e8b
Attachment #8348956 - Flags: checkin?(adam) → checkin+
https://hg.mozilla.org/mozilla-central/rev/1470c1a77e8b
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Looks like people see this problem in the field.
bwc: what is your view on uplifting this fix?
Flags: needinfo?(docfaraday)
Bug 978719 is the report from the field.
It would be nice to uplift this, but it will either depend on bug 935723, or require a backport. But let's first verify that this is the issue seen in bug 978719.
Flags: needinfo?(docfaraday)
Duplicate of this bug: 978719
Ok, it looks like this is indeed what was observed in the field. We could uplift to 28 (currently in Beta), but the bar there is a bit higher than Aurora, and it would get us 6 weeks. Let me see how easy it is to backport.
Seems to apply cleanly. Doing a build just in case, but I can request approval once that's done. Needinfoing myself.
Flags: needinfo?(docfaraday)
Comment on attachment 8348956 [details] [diff] [review]
NrIceCtx::ice_completed is fired even on failed contexts, so do not mark such contexts as completed.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 

   Has always been present since webrtc landed (Bug 790517 for this file)

User impact if declined: 

   Webrtc apps could behave strangely in failure cases, and webrtc app developers are still filing bugs on this.

Testing completed (on m-c, etc.): 

   This was landed on 29, so has seen some use in the field already, as well as the usual test cases.

Risk to taking this patch (and alternatives if risky): 

   Basically none.

String or IDL/UUID changes made by this patch:

   None.
Attachment #8348956 - Flags: approval-mozilla-beta?
Flags: needinfo?(docfaraday)
Comment on attachment 8348956 [details] [diff] [review]
NrIceCtx::ice_completed is fired even on failed contexts, so do not mark such contexts as completed.

At this stage in Beta we can only take on the churn & risk of patches that are tracked and/or sec-critical.  This will have to ride the trains and given it's been around for so long there's no reason to believe that will be a problem.
Attachment #8348956 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
You need to log in before you can comment on or make changes to this bug.