Closed Bug 1293206 Opened 3 years ago Closed 3 years ago

Intermittent dom/media/tests/mochitest/test_peerConnection_basicAudioNATRelay.html | application crashed [@ libc-2.23.so + 0x35418]

Categories

(Core :: WebRTC, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox48 --- wontfix
firefox49 --- fixed
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: drno)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files)

Rank: 35
Priority: -- → P3
With just a quick glance, I'm not seeing why that assert would fail:

static void nr_socket_buffered_stun_connected_cb(NR_SOCKET s, int how, void *arg)
{
  nr_socket_buffered_stun *sock = (nr_socket_buffered_stun *)arg;
  int r, _status;

  assert(!sock->connected);
(In reply to Byron Campen [:bwc] from comment #1)
> With just a quick glance, I'm not seeing why that assert would fail:

My only explanation for that would be that we get multiple callbacks for connected. Obviously as we haven't this before my patch in bug 1290365 is the most likely culprit.
Comment on attachment 8779562 [details]
Bug 1293206: cancel TCP connect callback after connect.

https://reviewboard.mozilla.org/r/70532/#review68012
Attachment #8779562 - Flags: review?(docfaraday) → review+
check back on the treeherder result.
Flags: needinfo?(drno)
Pushed by drno@ohlmeier.org:
https://hg.mozilla.org/integration/autoland/rev/30c27c9a8365
cancel TCP connect callback after connect. r=bwc
Flags: needinfo?(drno)
Assignee: nobody → drno
https://hg.mozilla.org/mozilla-central/rev/30c27c9a8365
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Blocks: 1290365
Same patch as the original on which landed in 51, just adapted to 50 and 49, so we can request uplift to unblock uplifting of bug 1290365.
Attachment #8781185 - Flags: review?(docfaraday)
Attachment #8781185 - Flags: review?(docfaraday) → review?(mfroman)
Attachment #8781185 - Flags: review?(mfroman) → review+
Comment on attachment 8781185 [details] [diff] [review]
bug1293206_aurora.patch

Approval Request Comment
[Feature/regressing bug #]: The original bug 891551 which added support for TCP ICE candidates already had the problem that the code relied on pending data for the TCP connection to replace the connect callback with the write callback. But we did not have proper testing which exposed the problem.

[User impact if declined]: The user impact of the wrong behavior is not really know, but this patch blocks the landing/uplifting for bug 1290365, which results in a small percentage of WebRTC users not being able to place successful calls when it should be possible for them.

[Describe test coverage new/current, TreeHerder]: The patch itself make our existing test work properly, so no additional test coverage needed.

[Risks and why]: The risk should be small as the same pattern of first canceling the callback before setting it again is already successfully used in several other place in the nICEr code base.

[String/UUID change made/needed]: N/A
Attachment #8781185 - Flags: approval-mozilla-beta?
Attachment #8781185 - Flags: approval-mozilla-aurora?
Comment on attachment 8781185 [details] [diff] [review]
bug1293206_aurora.patch

Has stabilized on Nightly51 for a few days, fixes an intermittent, worth uplifting to Aurora50.
Attachment #8781185 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8781185 [details] [diff] [review]
bug1293206_aurora.patch

Should be helpful for WebRTC, let's take it on beta 5
Attachment #8781185 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.