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

fix file descriptor leak in example http server code #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adamyi
Copy link

@adamyi adamyi commented Jul 6, 2020

Originally the code only closed the fd for the connection but not for the file.

I'm running this code in prod and it ran out of fds fairly quickly. This fix makes it more stable :)

@adamyi
Copy link
Author

adamyi commented Mar 23, 2022

Hey @ajyoon, just a friendly ping on this :)

Copy link
Owner

@ajyoon ajyoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apologies for my delay in patching in this very important production server fix. It seems correct enough to me, but I have a commentary question

Comment on lines +561 to +567
Move to cell 51
>>>

Close the opened file

The current cell is 49
The source file descriptor is in cell 53
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment above says it moves to cell 51, but this comment here says it's 49?

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