-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow the dev server socket to be reused immediately #4709
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
9 Ignored Deployments
|
// Allow the socket to be reused immediately after closing. This ensures that | ||
// the dev server can be restarted on the same address without a buffer time for | ||
// the OS to release the socket. | ||
let _ = socket.set_reuse_address(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will also allow binding to the same port twice, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pretty sure this will break our find_port
function
https://github.com/vercel/next.js/blob/3bdaed01ba9e1a86a16fd243e5bdb5e4894ed5aa/packages/next-swc/crates/next-dev/src/lib.rs#L173-L173
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's SO_REUSEPORT
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This indicates that futher calls to bind may allow reuse of local addresses. For IPv4 sockets this means that a socket may bind even when there's a socket already listening on this port
from the rustdoc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and it also says linux now matches this behavior
I'm fine with merging this for macOS/Linux only, but we might have to update the port finding code, we've already had a bunch of bug reports because of it not reporting ports in use correctly / not finding a new port as expected in the past and this might make it worse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this may very well break find_port
on macos and linux
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll test find_port
on macOS with and without this option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
find_port
still works properly with this option on macOS. However, as you said, binding to different local addresses no longer produces an error.
One way to fix this would be to replace find_port
to run lsof
/other OS-specific commands to check if a port is in use instead of trying to bind to that port.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to match the old behavior from before #4565: https://github.com/rust-lang/rust/blob/9c044d77a334609513f3b615e0763a40cc02424d/library/std/src/sys_common/net.rs#L399-L407
✅ This change can build |
🟢 CI successful 🟢Thanks |
### What? * vercel/turborepo#4700 <!-- Tobias Koppers - update deps --> * vercel/turborepo#4706 <!-- Tobias Koppers - make library code less verbose in stack traces --> * vercel/turborepo#4705 <!-- Tobias Koppers - improve error handling in update stream --> * vercel/turborepo#4667 <!-- Caleb Webber - remove box_syntax --> * vercel/turborepo#4714 <!-- Tobias Koppers - chunk hash need to include availability root --> * vercel/turborepo#4709 <!-- Alex Kirszenberg - Allow the dev server socket to be reused immediately --> * vercel/turborepo#4716 <!-- Tobias Koppers - errors lead to consistent exit code in issue detail -->
…o#4709) ### Description This avoids running into "Address already in use (os error 48)" when restarting the dev server in quick succession. --------- Co-authored-by: Tobias Koppers <[email protected]>
…o#4709) ### Description This avoids running into "Address already in use (os error 48)" when restarting the dev server in quick succession. --------- Co-authored-by: Tobias Koppers <[email protected]>
…o#4709) ### Description This avoids running into "Address already in use (os error 48)" when restarting the dev server in quick succession. --------- Co-authored-by: Tobias Koppers <[email protected]>
Description
This avoids running into "Address already in use (os error 48)" when restarting the dev server in quick succession.