get-free-port: prevent duplicate ports on Linux #564
Conversation
mjameswh
changed the title
There was a problem hiding this comment.
Do we need a new package instead of just putting this file back where the file that was deleted was (and add the test file next to it)? I don't mind that much though, but if you do move, calling devserver.CheckPortFree from the command is ok. Your choice.
There was a problem hiding this comment.
No, I can move it back. I originally moved it to a different package when I introduced freeport_windows.go vs freeport_darwin.go vs …, but since I reduced all of that to a single file, there no longer a justification for this.
| "runtime" | ||
| ) | ||
|
|
||
| /** |
There was a problem hiding this comment.
This is not the common format of Godoc, it's just // -prefixed lines
temporalcli/devserver/freeport.go
Outdated
| // in this regard; on that platform, `SO_REUSEADDR` has a different meaning and | ||
| // should not be set (setting it may have unpredictable consequences). | ||
| func GetFreePort(host string) (int, error) { | ||
| l, err := net.Listen("tcp", "127.0.0.1:0") |
There was a problem hiding this comment.
| l, err := net.Listen("tcp", "127.0.0.1:0") | |
| l, err := net.Listen("tcp", host+":0") |
I wonder if this makes more sense. Not that it matters much.
There was a problem hiding this comment.
It does. Good catch.
Co-authored-by: Chad Retz <chad.retz@gmail.com>
What changed
When assigning ephemeral ports for use by the Temporal Server, on all platforms except MacOS and Windows, immediately open a connection to that port, and force the connection into
TIME_WAITstate. This avoids various race conditions that could happen due to the port being reallocated between the moment the port is obtained and the moment the server actually binds to that port. Resolves [Feature Request] Fix free-port assignment to actually try to connect #550.Add checks to assert that ports provided by the user are indeed free; this would previously cause a SIGSEGV. Resolves SIGSEGV when using
--http-portoption for dev server and port is already in use #543.Added tests to confirm that it is possible to start multiple dev server concurrently.