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

Close the TCP socket explicitly #269

Merged
merged 1 commit into from
Oct 4, 2022
Merged

Close the TCP socket explicitly #269

merged 1 commit into from
Oct 4, 2022

Conversation

philippeapril
Copy link
Contributor

I had max open file problems on Mac, but I suppose the problem is on other platforms too.

I'm not familiar with the TCPSocket implementation but it looks like the socket is not being released if it's not explicitly closed, I read that it will be closed when it'll be read from or written to (and catch EOF or something like that), but it never seems to happen and the socket stays in a CLOSED state forever.

The way to reproduce was pretty much (in Rails console):

browser = Puppeteer.launch
browser.close

Then leave the Rails console open and run "lsof -n" on the PID of the Rails console and there will be a CLOSED TCP socket open like this:

ruby    26090 user   17u   IPv4 0xc6ba21f876ff5cbf      0t0                 TCP 127.0.0.1:60104->127.0.0.1:60103 (CLOSED)

If you start more in the same session, they'll pile up.

I'm not sure that there's a way to add an rspec test for this without lsof.

The TCP socket does not seem to be released properly and leaks, leading to max open files problems
@philippeapril philippeapril marked this pull request as ready for review October 3, 2022 21:09
@YusukeIwaki YusukeIwaki merged commit c3552c0 into YusukeIwaki:main Oct 4, 2022
@YusukeIwaki
Copy link
Owner

@philippeapril Thank you very much for your contribution. puppeteer-ruby 0.44.1 would fix this issue.

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