Test that server socket cannot reuse port

After removing allowReuseAddress and always setting SO_REUSEADDR, we need a test to ensure that the ports are not reused on *nix systems.

BUG=173533
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=215242

[Bei Zhang, 2013-07-08 18:08:25]

After removing allowReuseAddress and always setting SO_REUSEADDR, we need a test
to ensure that the ports are not reused on *nix systems.

[Bei Zhang, 2013-07-08 18:12:09]

Hi Wan-Teh,

Please review this test case for the remove of allowReuseAddress.

Thanks,
Bei

[wtc, 2013-07-25 18:03:56]

Review comments on patch set 1:

We need to make sure this test doesn't become flaky when
other users may be running the same test on the same machine
(such as a try bot) at the same time.

https://codereview.chromium.org/18683002/diff/2001/net/socket/tcp_server_sock...
File net/socket/tcp_server_socket_unittest.cc (right):

https://codereview.chromium.org/18683002/diff/2001/net/socket/tcp_server_sock...
net/socket/tcp_server_socket_unittest.cc:252: ParseAddress("127.0.0.1",
kListenPort, &address);

Using a constant listen port may prevent another person from running
this unit test on the same machine at the same time. This could be
problematic for a shared machine such as a try bot.

I suggest the following solution:

1. Have |socket| bind to port 0, and call socket.GetLocalAddress
to get the actual port number. Then, have |conflict_socket| bind to
that port number.

This still leaves a small window (after |socket.reset(NULL)| and before
the unit test ends) when |socket| releases the port, and another user
who runs the same test may grab that port.

2. Use a port number based on process id (PID), so that different
processes will use different port numbers.

3. Use a random port number. There is still a small probability that
the port numbers will collide.

[Bei Zhang, 2013-07-26 05:10:40]

Thanks for the review.

https://codereview.chromium.org/18683002/diff/2001/net/socket/tcp_server_sock...
File net/socket/tcp_server_socket_unittest.cc (right):

https://codereview.chromium.org/18683002/diff/2001/net/socket/tcp_server_sock...
net/socket/tcp_server_socket_unittest.cc:252: ParseAddress("127.0.0.1",
kListenPort, &address);
On 2013/07/25 18:03:56, wtc wrote:
> 
> Using a constant listen port may prevent another person from running
> this unit test on the same machine at the same time. This could be
> problematic for a shared machine such as a try bot.
> 
> I suggest the following solution:
> 
> 1. Have |socket| bind to port 0, and call socket.GetLocalAddress
> to get the actual port number. Then, have |conflict_socket| bind to
> that port number.
> 
> This still leaves a small window (after |socket.reset(NULL)| and before
> the unit test ends) when |socket| releases the port, and another user
> who runs the same test may grab that port.
> 
> 2. Use a port number based on process id (PID), so that different
> processes will use different port numbers.
> 
> 3. Use a random port number. There is still a small probability that
> the port numbers will collide.

Done. I think no solution is perfect here. I cannot really assure that the port
number derived from PID will not duplicate as well, it might also conflict with
existing program.

[wtc, 2013-07-26 21:24:21]

Patch set 2 LGTM.

https://codereview.chromium.org/18683002/diff/12001/net/socket/tcp_server_soc...
File net/socket/tcp_server_socket_unittest.cc (right):

https://codereview.chromium.org/18683002/diff/12001/net/socket/tcp_server_soc...
net/socket/tcp_server_socket_unittest.cc:252: // Bind to an random, unused port.

an => a

https://codereview.chromium.org/18683002/diff/12001/net/socket/tcp_server_soc...
net/socket/tcp_server_socket_unittest.cc:260: socket->GetLocalAddress(&address);

Check the return value:

  ASSERT_EQ(OK, socket->GetLocalAddress(&address));

https://codereview.chromium.org/18683002/diff/12001/net/socket/tcp_server_soc...
net/socket/tcp_server_socket_unittest.cc:266: // Unbind

Nit: add a period (.).

[Bei Zhang, 2013-08-01 20:14:40]

Thanks for the review!
Sorry for not replying timely.

https://codereview.chromium.org/18683002/diff/12001/net/socket/tcp_server_soc...
File net/socket/tcp_server_socket_unittest.cc (right):

https://codereview.chromium.org/18683002/diff/12001/net/socket/tcp_server_soc...
net/socket/tcp_server_socket_unittest.cc:252: // Bind to an random, unused port.
On 2013/07/26 21:24:21, wtc wrote:
> 
> an => a

Done.

https://codereview.chromium.org/18683002/diff/12001/net/socket/tcp_server_soc...
net/socket/tcp_server_socket_unittest.cc:260: socket->GetLocalAddress(&address);
On 2013/07/26 21:24:21, wtc wrote:
> 
> Check the return value:
> 
>   ASSERT_EQ(OK, socket->GetLocalAddress(&address));

Done.

https://codereview.chromium.org/18683002/diff/12001/net/socket/tcp_server_soc...
net/socket/tcp_server_socket_unittest.cc:266: // Unbind
On 2013/07/26 21:24:21, wtc wrote:
> 
> Nit: add a period (.).

Done.

[wtc, 2013-08-01 22:32:35]

Patch set 3 LGTM.

[commit-bot: I haz the power, 2013-08-02 00:18:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ikarienator@chromium.org/18683002/17001

[commit-bot: I haz the power, 2013-08-02 08:30:18]

Change committed as 215242