use port 0 rather than Net::EmptyPort

[karenetheridge]

from #plack:

13:56 < ether> hmm, it looks like Test::TCP is looking for an empty port.. but because this is an active server, the port it selected becomes unavailable before it can start the test
13:56 < ether> at least, this is what it looks like
13:56 < ether> perhaps there could be a way to disable tests that actively use the network?
13:57 < ether> or, loop until a free port is really found
13:57 < leedo> ether: yeah, i'm confused by Test::TCP doesn't bind to 0. i think that chooses a random open port
13:57 < leedo> maybe it isn't cross platform, though
13:59 < leedo> last i looked it did a while loop looking for open ports
14:19 < sri> this works pretty well in mojolicious so far for finding a free port https://github.com/kraih/mojo/blob/master/lib/Mojo/IOLoop/Server.pm#L100
14:20 < sri> pretty sure it actually predates Test::TCP
14:22 < sri> think there has been exactly one bug report over the years, and that was some broken version of cygwin... (more broken than usual)
14:24 < sri> (mojolicious always runs a few thousand tests that actively use the network)
14:26 < sri> oops, i misunderstood the problem... ignore me :)
14:26 < sri> or kinda...
14:27 < sri> bind to random port has the advantage that the operating system puts a cooldown on it, so it's not reused right away
14:27 < sri> misunderstood the problem... but the solution is still valid ;p
14:29 < sri> Net::EmptyPort is just weird
14:34 < leedo> yes, i'd be curious to see the reason for some of that

[miyagawa]

binding port 0 is a reliable solution but only works if you can control both the server and client.

Test::TCP needs to figure out an open port, then pass the port to a server process and a client, so that they can use the number as a command line options (on the server, like memcached -p $PORT) and then connect to it from the client.

[karenetheridge]

14:51 @miyagawa binding to 0 isn't the right solution for Test::TCP
14:51 @miyagawa because Test::TCP doesn't know which process it runs
14:52 @miyagawa it has to know which port the server should run, and then tell them to both client and server
14:52 < ether> ah that makes sense
14:52 < sri> still works though, see my solution
14:53 < ether> I wish it could use a file-based unix socket instead, and then we can just use File::Temp instead to generate a unique "port"
14:53 < ether> +domain

[mattn]

Excuse me for cutting in. I can't understand why @miyagawa don't approve this suggestion.

[miyagawa]

I'm not disapproving the suggestion. Just "always binding to 0 to let OS figure out the right port" doesn't work for Test::TCP's calling pattern since it has to acquire the port and then distributing them to both server/clients.

If the problem is that after looking for an open port and then there's a race condition between the port discovery and actual server binding, that problem will still exist no matter how we find out the open port.

That being said, binding to 0, then printing the sockport it bound to, sounds a lot simpler than randomly selecting a port and connecting to it to see if it's available.

[mattn]

make sense. Changes to use binding to 0 is a new feature, not a solution of this issue maybe. So it should be separated in this issue.

[dolmen]

More info about port 0: https://www.dnorth.net/2012/03/17/the-port-0-trick/ getsockname allows to retrieve the real port allocated.

[JJ]

So where does this stand?

[kazuho]

How about closing the issue?

While I agree that we could use port 0 for finding an empty port number within the ephemeral port range, the current approach works fairly well. I wonder if there is any practical reason to change how it is implemented.

PS. This is kind of a nit-pick, but I wonder if using one of the ephemeral ports is (was) a good approach. Maybe we use a port below 49152 so that the returned port would not overlap with one automatically allocated by other programs (that request an arbitrary port number by specifying zero).

[bjakubski]

I think that this known race condition (which is impossible to get rid of because of how API is designed) should get prominent notice in docs. I believe there are a lot of people not really familiar with sockets/TCP that just assume empty_port just reserves port for them. Probably it should mention that for reliable random port assignment "port 0 trick" (if possible to use) is better. Possibly unix domain sockets should also get a mention (great solution for testing when client and server allow that.

[kazuho]

@bjakubski

I think that this known race condition (which is impossible to get rid of because of how API is designed) should get prominent notice in docs.

Agreed. That is definitely something nice to have.

Probably it should mention that for reliable random port assignment "port 0 trick" (if possible to use) is better.

IMO, if it is a better approach, then we should switch to it. If it is not, we should add such an documentation. And my understanding regarding the issue is that using ephemeral ports for the purpose is not a good idea, as pointed out in my previous comment.

Possibly unix domain sockets should also get a mention (great solution for testing when client and server allow that.

I would suspect that nobody would argue against adding support for unix-sockets to Test::TCP.

[bjakubski]

@kazuho My comments were from perspective someone using Net::EmptyPort to set up some test and temporary instance of some service (as seen @work)

In such case if service being set up allows UNIX domain socket OR allows specifying port 0 these are always better options to get it running reliably. Using Net::EmptyPort (or similar) solution on busy test server may trigger its inherent race condition quite easily (and a bit of nondeterminism in test suite is not fun).

Of course this has little to no applicability to Test::TCP itself (with current API and it is for TCP and not unix domain sockets after all).

BTW it seems that on linux now the usual range for ephemeral ports is 32768-61000.

I agree that this issue can be closed. If there is such need someone might create issue to use "port 0" inside EmptyPort (not "instead" as in this issue)

[eserte]

I wonder if the test failures in the Test::TCP testsuite itself are also triggered by the same kind of race condition. See http://analysis.cpantesters.org/reports_by_field?SUBMIT_xxx=Submit&distv=Test-TCP-2.14&field=fail%3At%2F11_net_empty_port.t&field=qr%3A%28Address+already+in+use%29&field=conf%3Aarchname%2Bosvers&field=conf%3Aarchname%2Bosvers&field=meta%3Afrom&field=qr%3A%28Address+already+in+use%29

[nponeccop]

What if we only use the port 0 trick with listen => 1? The following example from the man page has a race now but can in principle be made race free:

test_tcp(
    listen => 1,
    client => sub {
        my $port = shift;
        # send request to the server
    },
    server => sub {
        my $socket = shift;
        # run server
    },
    # optional
    host => '127.0.0.1', # specify '::1' to test using IPv6
    port => 8080,
    max_wait => 3, # seconds
);

[nponeccop]

Apparently Net::EmptyPort::listen_socket() already uses port 0: undef is passed to _listen_socket() which is then converted to 0