Skip to content

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Jan 23, 2018

lchown and lchownSync were opening file descriptors without closing them. Looks like it has been that way for 7 years. Does anyone actually use these functions? :-)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

fs

lchown and lchownSync were opening file descriptors without
closing them. Looks like it has been that way for 7 years.
Does anyone actually use these functions?
@jasnell jasnell added the fs Issues and PRs related to the fs subsystem / file system. label Jan 23, 2018
@joyeecheung
Copy link
Member

@jasnell
Copy link
Member Author

jasnell commented Jan 24, 2018

Failures in CI are unrelated.

jasnell added a commit that referenced this pull request Jan 26, 2018
lchown and lchownSync were opening file descriptors without
closing them. Looks like it has been that way for 7 years.
Does anyone actually use these functions?

PR-URL: #18329
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@jasnell
Copy link
Member Author

jasnell commented Jan 26, 2018

Landed in 082f952

@jasnell jasnell closed this Jan 26, 2018
evanlucas pushed a commit that referenced this pull request Jan 30, 2018
lchown and lchownSync were opening file descriptors without
closing them. Looks like it has been that way for 7 years.
Does anyone actually use these functions?

PR-URL: #18329
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorins
Copy link
Contributor

MylesBorins commented Feb 27, 2018

I've landed on 8.x, but I'm not sure we need this on 6.x

It looks like an optimization in #14055 may have created the leak

As you can see in 6.x, the fd is always closed (or at least it is attempted).

edit: seeing there were other leaks, probably want to land that on v6.x, @jasnell would you open a backport?

MylesBorins pushed a commit that referenced this pull request Feb 27, 2018
lchown and lchownSync were opening file descriptors without
closing them. Looks like it has been that way for 7 years.
Does anyone actually use these functions?

PR-URL: #18329
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
lchown and lchownSync were opening file descriptors without
closing them. Looks like it has been that way for 7 years.
Does anyone actually use these functions?

PR-URL: nodejs#18329
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants