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

close free sockets#18

Merged
TooTallNate merged 1 commit intoTooTallNate:masterfrom
tareksha:patch-1
Jul 5, 2018
Merged

close free sockets#18
TooTallNate merged 1 commit intoTooTallNate:masterfrom
tareksha:patch-1

Conversation

@tareksha
Copy link
Copy Markdown
Contributor

@tareksha tareksha commented Jul 2, 2018

The issue is discussed thoroughly here
https://github.com/zkat/pacote/issues/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
Copy Markdown
Owner

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

@tareksha
Copy link
Copy Markdown
Contributor Author

tareksha commented Jul 2, 2018

@TooTallNate fixed :)

@TooTallNate
Copy link
Copy Markdown
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
Copy Markdown

mk-pmb commented Jul 3, 2018

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

@tareksha
Copy link
Copy Markdown
Contributor Author

tareksha commented Jul 4, 2018

@TooTallNate done

@tareksha
Copy link
Copy Markdown
Contributor Author

tareksha commented Jul 5, 2018

any decisions here?

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

Merged and released in v4.2.1. Thanks!

@mk-pmb
Copy link
Copy Markdown

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