-
-
Notifications
You must be signed in to change notification settings - Fork 64
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 in-process race condition #36
Conversation
629710a
to
6198da5
Compare
6198da5
to
57c443b
Compare
Thanks for working on this 🙌 Can you add a test? |
Co-Authored-By: Sindre Sorhus <sindresorhus@gmail.com>
thanks @sindresorhus for the feedback, should be all addressed now 👍 oops - except for test, incoming |
@sindresorhus ok test added, also realised that we only need one setInterval and it's lazily initialised |
The code looks good. Can you mention the behavior in the readme? That ports are locked up, how long, why, and how it helps prevent race-conditions. |
@sindresorhus description of behaviour has been added to the readme 👍 |
Excellent. Thanks for working on this :) |
thanks @sindresorhus ! |
This is related to #23 but it's doesn't solve the multiprocess race condition.
This might have only started occurring in later node versions, but when calling
get-port
a lot sometimes the same port will be returned and then port already bound error occurs when trying to bind to it.You can extend this to solve #23 by replacing
Set
with an ondisk store (maybe leveldb?).The reason for using an old/young sets approach is to avoid setting one interval per port (which would be overkill imo) while also ensuring that a port isn't forgotten for at least the
sweep
time, but up to double thesweep
time.