Skip to content
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

fix(ext/node): enable reuseport for net Socket on macOS #20869

Closed
wants to merge 5 commits into from

Conversation

littledivy
Copy link
Member

@littledivy littledivy commented Oct 10, 2023

  • Enables SO_REUSEPORT on macOS
  • All Node net.Socket TCP sockets are set to SO_REUSEPORT.

Fixes #20618
Fixes #18301

Modules working: npm:http-server, npm:portfinder and Vuepress dev server

ext/net/01_net.js Outdated Show resolved Hide resolved
Copy link
Member

@lucacasonato lucacasonato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does SO_REUSEPORT do on macOS? When I originally implemented this for Linux, I didn't do macOS because there was no reason for anyone to use it (because it behaves different on macOS - it steals rather than balances). What do these packages use it for?

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@lucacasonato lucacasonato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I think you don't want SO_REUSEPORT, but SO_REUSEADDR (and enable that by default everywhere). libuv does not enable (or let you enable) SO_REUSEPORT on TCP sockets anywhere (including macOS). It does enable SO_REUSEADDR on all Unix.

@littledivy
Copy link
Member Author

@lucacasonato is right. libuv does not set SO_REUSEPORT for TCP (UDP only on BSDs)

It looks like portfinder is also flaky/broken in Node -

@bartlomieju How should we proceed?

@bartlomieju
Copy link
Member

@littledivy I'm not sure yet - but given this package is popular we need to come up with some solution that will work in most cases (compared to always crashing right now).

@littledivy
Copy link
Member Author

littledivy commented Mar 5, 2024

Closing because stale.

@littledivy littledivy closed this Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support VuePress (dev server) node: Error when using npm:portfinder
3 participants