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.

Sign up for GitHub

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

Jump to bottom

Explicitly open/close files to resolve EMFILE #3021

Closed
wants to merge 1 commit into from
Closed

Explicitly open/close files to resolve EMFILE #3021

wants to merge 1 commit into from

Conversation

sam-codaio
Copy link
Contributor

  • Attempt to resolve EMFILE errors by explicitly closing files instead of relying on readFileSync

@sam-codaio sam-codaio requested review from a team as code owners July 17, 2024 16:26
dweitzman-codaio
dweitzman-codaio reviewed Jul 17, 2024
View reviewed changes
runtime/bootstrap/index.ts

// readFileSync does not properly close files in the lambda
const fileDescriptor = fs.openSync(path, 'r');
const bundle = fs.readFileSync(fileDescriptor).toString();
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the readFileSync implementation already calls closeSync: https://github.com/nodejs/node/blob/589b67aa1748974b24df4b4b7281e0cce4e915c7/lib/fs.js#L481

Copy link
Contributor

Choose a reason for hiding this comment

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

@sam-codaio mentioned that there are reports of it not closing properly in lambda. This explicit call will close it is that right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it should be closed by the implementation

Copy link
Contributor

Choose a reason for hiding this comment

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

If there a stack overflow or something about a lambda-specific bug the code can link to?

@sam-codaio sam-codaio closed this Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dweitzman-codaio dweitzman-codaio dweitzman-codaio left review comments

@johnli-codaio johnli-codaio johnli-codaio left review comments

@gary-codaio gary-codaio gary-codaio approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sam-codaio @johnli-codaio @dweitzman-codaio @gary-codaio