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

Pool getPort race condition #14

Merged
merged 2 commits into from
Nov 24, 2019
Merged

Conversation

jogli5er
Copy link
Contributor

It appears that when requesting multiple ports asynchronously,
that a port is given out multiple times, which crashes single
Tor processes. Such a crash brings down the whole system as of
now, which might or might not be desired.
This is caused by a race condition on the port numbers. This
commit fixes the issue if a number of Tor instances for the pool
is requested. If the pool is populated with an array of definitions,
the user has to make sure to include the ports in the definition,
otherwise the race condition still persists. If this should still be
allowed is up for discussion or if a check should be included in the
TorPool create function.

It appears that when requesting multiple ports asynchronously,
that a port is given out multiple times, which crashes single
Tor processes. Such a crash brings down the whole system as of
now, which might or might not be desired.
This is caused by a race condition on the port numbers. This
commit fixes the issue if a number of Tor instances for the pool
is requested. If the pool is populated with an array of definitions,
the user has to make sure to include the ports in the definition,
otherwise the race condition still persists. If this should still be
allowed is up for discussion or if a check should be included in the
TorPool create function.
@jogli5er
Copy link
Contributor Author

jogli5er commented Nov 21, 2019

@znetstar can you review the changes?

@znetstar
Copy link
Owner

Looks good to me. Thanks for finding a solution!

@znetstar znetstar merged commit 37a58f9 into znetstar:master Nov 24, 2019
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.

2 participants