Skip to content

No closeSync API called #2

Open

Description

@xiabin1235910

I found in helper.js, we open the file to get the fileDescriptor using fs.openSync(filePath, 'r').
But do we need to close it when the server is closed?

the code in the node official API

server.on('close', () => fs.closeSync(fd));

The link is
https://nodejs.org/dist/latest-v9.x/docs/api/http2.html#http2_http2stream_respondwithfd_fd_headers_options

Activity

  1. xiabin1235910 commented on Nov 8, 2017

    @xiabin1235910
    Author

    like in helper.js

    function closeFiles (baseDir) {
        ...
    }
    
  2. Shadowbeetle commented on Apr 24, 2018

    @Shadowbeetle
    Member

    Sorry for the late response. In case you're still interested:

    That could be a problem if we wrote to the files, or if we opened many different files for different endpoints and ran out of Unix sockets. Neither if the two happens in this.

    This solution is a bit more efficient as we open the files when the server is fired up, so we can read the file contents straight away upon receiving a request. You are right, however, that it would be safer and nicer to only open the files when they are needed and close them once their content is sent.

    Would you be up for opening a pull request to fix this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions