server: FreeTCPAddr is brittle by assuming that closed formerly bound sockets can't be reused fast enough

[odeke-em]

Noticed via audit, the code in FreeTCPAddr asks the system for any available port by

ln, err := net.Listen("tcp", "localhost:0")

defer ln.Close()

portI := l.Addr().(*net.TCPAddr).Port
port := fmt.Sprintf("%d", portI)
addr := fmt.Sprintf("tcp://0.0.0.0:%s", port)

but that close until the underlying system reclaims closed ports is a shot in the dark. This can introduce non-determinism
if trying to create a server that way.

If this function isn't being used, let's decide what to do with it. Otherwise, we need to figure out how to properly obtain and
use that listener without having to close it immediately. I work on a project where reuse of file descriptors
and sockets is a huge pain given the huge number of tests that we run a day, and if dealing with Byzantine actors, this
could be a potential blindspot.

[alexanderbez]

Thanks for raising an issue @odeke-em. If you do a quick grok of the codebase, you'll see we absolutely do use and depend on FreeTCPAddr. Primarily in integration testing. We need this function. However, there are no "Byzantine" actors. Just faulty tests sometimes if a port is already taken.

Feel free to open a PR with a revised implementation 👍

[alexanderbez]

Took a look at https://github.com/phayes/freeport which seems to do the exact same thing we do. So what exactly is the issue and what is your proposal @odeke-em?

[odeke-em]

Took a look at https://github.com/phayes/freeport which seems to do the exact same thing we do. So what exactly is the issue and what is your proposal @odeke-em?

It is not blaming anyone or anything, I was just raising a problem that's common even in that library and that many people gloss over (which doesn't mean that it is alright to do, it can be incorrect even if the whole world does it) that when looking for addresses to run on, we first bind to an available port and then close it, hoping that it won't be reused. It is the liberty of the OS to reuse closed ports.

My proposal is that perhaps a sentinel value when we don't know the address to bind to, and then when a server is being spun up, we recognize the sentinel value and then create the listener and bind to it. That last step is done anyways if am not mistaken, just that it was done twice.

[alexanderbez]

All good!

A sentinel value? How would that work with picking a random available port? Can you post some pseudo-code perhaps?

[tac0turtle]

closing this as its located in a testing file now. Developers should be aware its only for testing now