Closed Bug 1135405 Opened 5 years ago Closed 5 years ago

Fix netwerk/test/unit/test_udp_multicast.js

Categories

(Core :: Networking, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox38 --- fixed
firefox39 --- fixed
b2g-v1.4 --- unaffected
b2g-v2.0 --- fixed
b2g-v2.0M --- fixed
b2g-v2.1 --- fixed
b2g-v2.1S --- fixed
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: dragana, Assigned: dragana)

References

Details

Attachments

(1 file, 1 obsolete file)

The test is failing some time.
The last test is failing, the test with leaving multicast group.
If a socket leaves a group but stays bound to the same port and there are other sockets on the host joined to the same multicast group, the socket will still receive packets for that multicast group.

The socket from the test before the test with leaving the multicast group, sometimes is still active when this test starts and therefore the test is failing.
Attached patch bug_1135405_v1.patch (obsolete) — Splinter Review
Assignee: nobody → dd.mozilla
Status: NEW → ASSIGNED
Blocks: 1131557
Comment on attachment 8567977 [details] [diff] [review]
bug_1135405_v1.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bbd008ad6498

I have run it with patch from bug 1131557 because it was failing with it more often.
Attachment #8567977 - Flags: review?(michal.novotny)
Blocks: 1124880
(In reply to Dragana Damjanovic [:dragana] from comment #0)
> If a socket leaves a group but stays bound to the same port and there are
> other sockets on the host joined to the same multicast group, the socket
> will still receive packets for that multicast group.

Isn't this a bug that needs to be fixed instead of suppressing it by reordering the test?
Flags: needinfo?(dd.mozilla)
(In reply to Michal Novotny (:michal) from comment #3)
> (In reply to Dragana Damjanovic [:dragana] from comment #0)
> > If a socket leaves a group but stays bound to the same port and there are
> > other sockets on the host joined to the same multicast group, the socket
> > will still receive packets for that multicast group.
> 
> Isn't this a bug that needs to be fixed instead of suppressing it by
> reordering the test?

This behavior is not in firefox but in the kernel. 
The socket from the previous test is still not closed at the moment when we start this test. That is way this happens only sometimes and more often with the patch from bug 1131557.
Flags: needinfo?(dd.mozilla)
Comment on attachment 8567977 [details] [diff] [review]
bug_1135405_v1.patch

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

(In reply to Dragana Damjanovic [:dragana] from comment #4)
> This behavior is not in firefox but in the kernel. 
> The socket from the previous test is still not closed at the moment when we
> start this test. That is way this happens only sometimes and more often with
> the patch from bug 1131557.

It's still not clear to me whether this is an expected behavior or a bug. Anyway, better fix would be to use a different address for every test. Initializing UDPSocket with aAddressReuse == false should IMO work too.
Attachment #8567977 - Flags: review?(michal.novotny) → review-
(In reply to Michal Novotny (:michal) from comment #5)
> Comment on attachment 8567977 [details] [diff] [review]
> bug_1135405_v1.patch
> 
> Review of attachment 8567977 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> (In reply to Dragana Damjanovic [:dragana] from comment #4)
> > This behavior is not in firefox but in the kernel. 
> > The socket from the previous test is still not closed at the moment when we
> > start this test. That is way this happens only sometimes and more often with
> > the patch from bug 1131557.
> 
> It's still not clear to me whether this is an expected behavior or a bug.
> Anyway, better fix would be to use a different address for every test.
> Initializing UDPSocket with aAddressReuse == false should IMO work too.


It is expected behavior.
aAddressReuse can not work because it is not binding.
Attachment #8572547 - Flags: review?(michal.novotny) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f124f6d08ec7
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment on attachment 8572547 [details] [diff] [review]
bug_1135405_v2.patch

Approval Request Comment
[Feature/regressing bug #]: ?
[User impact if declined]: none, it's just a test change
[Describe test coverage new/current, TreeHerder]: in-testsuit+
[Risks and why]: :)
[String/UUID change made/needed]: none
Attachment #8572547 - Flags: approval-mozilla-beta?
Comment on attachment 8572547 [details] [diff] [review]
bug_1135405_v2.patch

Should be in 38 beta 3
Attachment #8572547 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Blocks: 1158189
No longer blocks: 1124880
You need to log in before you can comment on or make changes to this bug.