New issue
Advanced search Search tips

Issue 821516 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Mac , Fuchsia
Pri: 2
Type: Bug



Sign in to add a comment

UDPSocketTest.ConnectRandomBind flaked under Fuchsia/x64 FYI bot

Project Member Reported by w...@chromium.org, Mar 13 2018

Issue description

UDPSocketTest.ConnectRandomBind failed in https://build.chromium.org/p/chromium.fyi/builders/Fuchsia/builds/14763 with:

[00372.444] 03954.03997> [6364/24907] UDPSocketTest.Connect (56 ms)
[00372.444] 03954.03997> [6365/24907] UDPSocketTest.PartialRecv (51 ms)
[00372.445] 03954.03997> [ RUN      ] UDPSocketTest.ConnectRandomBind
[00372.445] 03954.03997> ../../net/socket/udp_socket_unittest.cc:407: Failure
[00372.446] 03954.03997> Expected equality of these values:
[00372.446] 03954.03997>   used_ports.back()
[00372.446] 03954.03997>     Which is: 22222
[00372.447] 03954.03997>   client_address.port()
[00372.447] 03954.03997>     Which is: 42336
[00372.447] 03954.03997> [  FAILED  ] UDPSocketTest.ConnectRandomBind (1287 ms)
[00372.448] 03954.03997> [6366/24907] UDPSocketTest.ConnectRandomBind (1287 ms)
[00372.448] 03954.03997> [6367/24907] UDPSocketTest.VerifyConnectBindsAddr (170 ms)

It is noticable that the ConnectRandomBind test took 1200ms to run in this failed run, whereas in the previous successful run it took only 250ms.

So perhaps the expectation is missed because part of the test timed-out after 1s?
 

Comment 1 by w...@chromium.org, Mar 13 2018

Cc: -sergeyu@chromium.org
Owner: sergeyu@chromium.org
Status: Assigned (was: Untriaged)
sergeyu: The test expectation mismatch doesn't really help diagnose what went wrong, unfortunately. Is it possible we're hitting the port-retention issue that you mentioned last week?

Plz take a look, and disable/filter the test if necessary - thanks!
Port retention issues was TCP-specific, I don't think it can affect UDP. This looks like something else.
Labels: OS-Linux OS-Mac
Looks like the test is broken. It connects a UDP socket, remembers local port number and closes the socket. Then it assumes that the port number will continue to be available for a new socket. This may not be true, e.g. if there are other UDP tests running in parallel on the same machine. UDPSocketPosix::RandomBind() calls bind(<addr>:0) if it fails to bind to a random port after 10 attempts, essentially failing over to the OS to allocate a local port. Looks like that's what happened in this case.
IMO the right solution would be to rewrite UDP tests avoid relying on fake RNG. AFAICT there are only 3 tests that use it and these are all in poor state:
 - UDPSocketTest.ConnectRandomBind is flaky
 - UDPSocketTest.ConnectFail - works only on some platforms
 - UDPSocketTest.TestBindToNetwork - platform dependencies. Doesn't really need fake RNG.
Also some platforms (e.g. Fuchsia) use random ports by default, so there is no need to generate port number explicitly in Chrome, but it's hard to avoid it currently because the tests rely on the fake RNG.
Removing rand_int_cb parameter from UDPSocket constructor will also make all calling code cleaner.
Status: Started (was: Assigned)
Pending fix: https://chromium-review.googlesource.com/c/chromium/src/+/964895
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cbdfc88589f8b37942549f5ff7225e5cf23b0198

commit cbdfc88589f8b37942549f5ff7225e5cf23b0198
Author: Sergey Ulanov <sergeyu@chromium.org>
Date: Fri Mar 16 20:13:28 2018

Remove RandIntCallback from UDPSocket parameters.

UDPSocket supports RANDOM_BIND flag which ensures that local port number
is allocated randomly. To implement this feature it was previously taking
RandIntCallback as one of constructor parameters. That parameter was
useful only in tests - in other cases base::RandInt() was always
sufficient. Than additional callback adds extra complexity where is can
be avoided. Also some platforms (e.g. Fuchsia) allocate port numbers
randomly by default. That callback made it impossible to rely on the OS
for port random allocation.

In this CL:
 1. Removed RandIntCallback from UDPSocket constructor. UDPSocket
    implementations are supposed to generate random port numbers
    internally.
 2. Updated unittests that were relying on this custom RandIntCallback.
    New RandomBind unittest should be more reliable (see linked bug).

TBR=miu@chromium.org, bbudge@chromium.org, mikhail.pozdnyakov@intel.com

Bug:  821516 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: If813d9db2ce5a5b0c6cdcfd8670361ccf6836fd8
Reviewed-on: https://chromium-review.googlesource.com/964895
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543801}
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/chrome/browser/media/router/discovery/dial/dial_service.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/content/browser/renderer_host/p2p/socket_dispatcher_host.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/content/browser/renderer_host/pepper/pepper_udp_socket_message_filter.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/extensions/browser/api/display_source/display_source_apitestbase.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/media/cast/net/udp_transport_impl.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/dns/address_sorter_posix.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/dns/address_sorter_posix_unittest.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/dns/dns_session_unittest.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/dns/dns_socket_pool.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/dns/dns_transaction_unittest.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/dns/host_resolver_impl.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/quic/chromium/quic_chromium_client_session_test.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/quic/chromium/quic_connectivity_probing_manager_test.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/quic/chromium/quic_stream_factory.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/quic/chromium/quic_stream_factory_test.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/client_socket_factory.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/client_socket_factory.h
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/client_socket_pool_base_unittest.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/fuzzed_socket_factory.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/fuzzed_socket_factory.h
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/socket_test_util.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/socket_test_util.h
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/transport_client_socket_pool_test_util.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/transport_client_socket_pool_test_util.h
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/udp_client_socket.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/udp_client_socket.h
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/udp_server_socket.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/udp_socket_perftest.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/udp_socket_posix.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/udp_socket_posix.h
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/udp_socket_unittest.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/udp_socket_win.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/socket/udp_socket_win.h
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/net/tools/quic/quic_client_message_loop_network_helper.cc
[modify] https://crrev.com/cbdfc88589f8b37942549f5ff7225e5cf23b0198/services/network/udp_socket.cc

Status: Fixed (was: Started)

Sign in to add a comment