fs: cleanup fd lchown and lchownSync

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]>

lib/fs.js

@@ -1330,20 +1330,16 @@ if (constants.O_SYMLINK !== undefined) {
   };
 
   fs.lchmodSync = function(path, mode) {
-    var fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);
+    const fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);
 
     // Prefer to return the chmod error, if one occurs,
     // but still try to close, and report closing errors if they occur.
-    var ret;
+    let ret;
     try {
       ret = fs.fchmodSync(fd, mode);
-    } catch (err) {
-      try {
-        fs.closeSync(fd);
-      } catch (ignore) {}
-      throw err;
+    } finally {
+      fs.closeSync(fd);
     }
-    fs.closeSync(fd);
     return ret;
   };
 }
@@ -1381,13 +1377,25 @@ if (constants.O_SYMLINK !== undefined) {
         callback(err);
         return;
       }
-      fs.fchown(fd, uid, gid, callback);
+      // Prefer to return the chown error, if one occurs,
+      // but still try to close, and report closing errors if they occur.
+      fs.fchown(fd, uid, gid, function(err) {
+        fs.close(fd, function(err2) {
+          callback(err || err2);
+        });
+      });
     });
   };
 
   fs.lchownSync = function(path, uid, gid) {
-    var fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);
-    return fs.fchownSync(fd, uid, gid);
+    const fd = fs.openSync(path, constants.O_WRONLY | constants.O_SYMLINK);
+    let ret;
+    try {
+      ret = fs.fchownSync(fd, uid, gid);
+    } finally {
+      fs.closeSync(fd);
+    }
+    return ret;
   };
 }