1135405 - Fix netwerk/test/unit/test_udp_multicast.js

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Dragana Damjanovic [:dragana]]

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.

Comment 1

[Dragana Damjanovic [:dragana]]

Attachment: bug_1135405_v1.patch

Comment 2

[Dragana Damjanovic [:dragana]]

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.

Comment 3

[Michal Novotny [:michal]]

(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?

Comment 4

[Dragana Damjanovic [:dragana]]

(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.

Comment 5

[Michal Novotny [:michal]]

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.

Comment 6

[Dragana Damjanovic [:dragana]]

Attachment: bug_1135405_v2.patch

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

Comment 7

[Dragana Damjanovic [:dragana]]

(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.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f124f6d08ec7

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f124f6d08ec7

Comment 10

[Honza Bambas (:mayhemer)]

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

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8572547 [details] [diff] [review]
bug_1135405_v2.patch

Should be in 38 beta 3

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8bb13d7a5d2a

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/99a0854a2f59
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/b3b3693a24bd
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/261da07117ef

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/b3b3693a24bd
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/261da07117ef