Skip to content

Commit 3e21dd9

Browse files
committedSep 27, 2020
src: add option to track unmanaged file descriptors
Add the ability to track “raw” file descriptors, i.e. integers returned by `fs.open()`, and close them on `Environment` shutdown, to match the behavior for all other resource types (which are also closed on shutdown). PR-URL: #34303 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
1 parent a5aa3dd commit 3e21dd9

File tree

6 files changed

+49
-2
lines changed

6 files changed

+49
-2
lines changed
 

‎src/env-inl.h

+4
Original file line numberDiff line numberDiff line change
@@ -871,6 +871,10 @@ inline bool Environment::owns_inspector() const {
871871
return flags_ & EnvironmentFlags::kOwnsInspector;
872872
}
873873

874+
inline bool Environment::tracks_unmanaged_fds() const {
875+
return flags_ & EnvironmentFlags::kTrackUnmanagedFds;
876+
}
877+
874878
inline uint64_t Environment::thread_id() const {
875879
return thread_id_;
876880
}

‎src/env.cc

+24
Original file line numberDiff line numberDiff line change
@@ -680,6 +680,12 @@ void Environment::RunCleanup() {
680680
}
681681
CleanupHandles();
682682
}
683+
684+
for (const int fd : unmanaged_fds_) {
685+
uv_fs_t close_req;
686+
uv_fs_close(nullptr, &close_req, fd, nullptr);
687+
uv_fs_req_cleanup(&close_req);
688+
}
683689
}
684690

685691
void Environment::RunAtExitCallbacks() {
@@ -1045,6 +1051,24 @@ Environment* Environment::worker_parent_env() const {
10451051
return worker_context()->env();
10461052
}
10471053

1054+
void Environment::AddUnmanagedFd(int fd) {
1055+
if (!tracks_unmanaged_fds()) return;
1056+
auto result = unmanaged_fds_.insert(fd);
1057+
if (!result.second) {
1058+
ProcessEmitWarning(
1059+
this, "File descriptor %d opened in unmanaged mode twice", fd);
1060+
}
1061+
}
1062+
1063+
void Environment::RemoveUnmanagedFd(int fd) {
1064+
if (!tracks_unmanaged_fds()) return;
1065+
size_t removed_count = unmanaged_fds_.erase(fd);
1066+
if (removed_count == 0) {
1067+
ProcessEmitWarning(
1068+
this, "File descriptor %d closed but not opened in unmanaged mode", fd);
1069+
}
1070+
}
1071+
10481072
void Environment::BuildEmbedderGraph(Isolate* isolate,
10491073
EmbedderGraph* graph,
10501074
void* data) {

‎src/env.h

+6
Original file line numberDiff line numberDiff line change
@@ -1066,6 +1066,7 @@ class Environment : public MemoryRetainer {
10661066
inline bool should_not_register_esm_loader() const;
10671067
inline bool owns_process_state() const;
10681068
inline bool owns_inspector() const;
1069+
inline bool tracks_unmanaged_fds() const;
10691070
inline uint64_t thread_id() const;
10701071
inline worker::Worker* worker_context() const;
10711072
Environment* worker_parent_env() const;
@@ -1277,6 +1278,9 @@ class Environment : public MemoryRetainer {
12771278
void RunAndClearNativeImmediates(bool only_refed = false);
12781279
void RunAndClearInterrupts();
12791280

1281+
void AddUnmanagedFd(int fd);
1282+
void RemoveUnmanagedFd(int fd);
1283+
12801284
private:
12811285
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
12821286
const char* errmsg);
@@ -1432,6 +1436,8 @@ class Environment : public MemoryRetainer {
14321436
ArrayBufferAllocatorList;
14331437
ArrayBufferAllocatorList* keep_alive_allocators_ = nullptr;
14341438

1439+
std::unordered_set<int> unmanaged_fds_;
1440+
14351441
std::function<void(Environment*, int)> process_exit_handler_ {
14361442
DefaultProcessExitHandler };
14371443

‎src/node.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,10 @@ enum Flags : uint64_t {
400400
// Set if Node.js should not run its own esm loader. This is needed by some
401401
// embedders, because it's possible for the Node.js esm loader to conflict
402402
// with another one in an embedder environment, e.g. Blink's in Chromium.
403-
kNoRegisterESMLoader = 1 << 3
403+
kNoRegisterESMLoader = 1 << 3,
404+
// Set this flag to make Node.js track "raw" file descriptors, i.e. managed
405+
// by fs.open() and fs.close(), and close them during FreeEnvironment().
406+
kTrackUnmanagedFds = 1 << 4
404407
};
405408
} // namespace EnvironmentFlags
406409

‎src/node_file.cc

+6
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,9 @@ void AfterInteger(uv_fs_t* req) {
604604
FSReqBase* req_wrap = FSReqBase::from_req(req);
605605
FSReqAfterScope after(req_wrap, req);
606606

607+
if (req->result >= 0 && req_wrap->is_plain_open())
608+
req_wrap->env()->AddUnmanagedFd(req->result);
609+
607610
if (after.Proceed())
608611
req_wrap->Resolve(Integer::New(req_wrap->env()->isolate(), req->result));
609612
}
@@ -813,6 +816,7 @@ void Close(const FunctionCallbackInfo<Value>& args) {
813816

814817
CHECK(args[0]->IsInt32());
815818
int fd = args[0].As<Int32>()->Value();
819+
env->RemoveUnmanagedFd(fd);
816820

817821
FSReqBase* req_wrap_async = GetReqWrap(env, args[1]);
818822
if (req_wrap_async != nullptr) { // close(fd, req)
@@ -1653,6 +1657,7 @@ static void Open(const FunctionCallbackInfo<Value>& args) {
16531657

16541658
FSReqBase* req_wrap_async = GetReqWrap(env, args[3]);
16551659
if (req_wrap_async != nullptr) { // open(path, flags, mode, req)
1660+
req_wrap_async->set_is_plain_open(true);
16561661
AsyncCall(env, req_wrap_async, args, "open", UTF8, AfterInteger,
16571662
uv_fs_open, *path, flags, mode);
16581663
} else { // open(path, flags, mode, undefined, ctx)
@@ -1662,6 +1667,7 @@ static void Open(const FunctionCallbackInfo<Value>& args) {
16621667
int result = SyncCall(env, args[4], &req_wrap_sync, "open",
16631668
uv_fs_open, *path, flags, mode);
16641669
FS_SYNC_TRACE_END(open);
1670+
if (result >= 0) env->AddUnmanagedFd(result);
16651671
args.GetReturnValue().Set(result);
16661672
}
16671673
}

‎src/node_file.h

+5-1
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ class FSReqBase : public ReqWrap<uv_fs_t> {
6666
const char* data() const { return has_data_ ? *buffer_ : nullptr; }
6767
enum encoding encoding() const { return encoding_; }
6868
bool use_bigint() const { return use_bigint_; }
69+
bool is_plain_open() const { return is_plain_open_; }
70+
71+
void set_is_plain_open(bool value) { is_plain_open_ = value; }
6972

7073
FSContinuationData* continuation_data() const {
7174
return continuation_data_.get();
@@ -87,8 +90,9 @@ class FSReqBase : public ReqWrap<uv_fs_t> {
8790
std::unique_ptr<FSContinuationData> continuation_data_;
8891
enum encoding encoding_ = UTF8;
8992
bool has_data_ = false;
90-
const char* syscall_ = nullptr;
9193
bool use_bigint_ = false;
94+
bool is_plain_open_ = false;
95+
const char* syscall_ = nullptr;
9296

9397
// Typically, the content of buffer_ is something like a file name, so
9498
// something around 64 bytes should be enough.

0 commit comments

Comments
 (0)
Please sign in to comment.