Skip to content
This repository has been archived by the owner on May 5, 2023. It is now read-only.

close free sockets #18

Merged
merged 1 commit into from
Jul 5, 2018
Merged

close free sockets #18

merged 1 commit into from
Jul 5, 2018

Conversation

tareksha
Copy link
Contributor

@tareksha tareksha commented Jul 2, 2018

The issue is discussed thoroughly here
zkat/pacote#138

The various fetching libraries signal free proxy sockets with free event. Close the socket instead of leaving them uncontrollably open and eventually exploding due to high number of open connections. This is a major blocker in some cases because some corporate proxies have hard limits on concurrently open connections.

@TooTallNate
Copy link
Owner

Makes sense. Thanks for the patch. Any idea why the tests are failing? 🤔

@tareksha
Copy link
Contributor Author

tareksha commented Jul 2, 2018

@TooTallNate fixed :)

@TooTallNate
Copy link
Owner

Thanks @tareqhs. Sorry for not mentioning this before, but is it possible in this case to add a regression test?

@mk-pmb
Copy link

mk-pmb commented Jul 3, 2018

@tareqhs Much thanks! You're my hero for today. 💯

@tareksha
Copy link
Contributor Author

tareksha commented Jul 4, 2018

@TooTallNate done

@tareksha
Copy link
Contributor Author

tareksha commented Jul 5, 2018

any decisions here?

@TooTallNate TooTallNate merged commit 783e956 into TooTallNate:master Jul 5, 2018
@TooTallNate
Copy link
Owner

Merged and released in v4.2.1. Thanks!

@mk-pmb
Copy link

mk-pmb commented Jul 27, 2020

So was this supposed to fix npm? Because with npm v6.14.6 on nodejs v12.18.3 on Ubuntu focal, I have the same issue again with maxsockets being ignored.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants