Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 3e21dd9

Browse filesBrowse files
addaleaxaddaleax
committed
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 <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent a5aa3dd commit 3e21dd9
Copy full SHA for 3e21dd9

File tree

Expand file treeCollapse file tree

6 files changed

+49
-2
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

6 files changed

+49
-2
lines changed
Open diff view settings
Collapse file

‎src/env-inl.h‎

Copy file name to clipboardExpand all lines: src/env-inl.h
+4Lines changed: 4 additions & 0 deletions
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
}
Collapse file

‎src/env.cc‎

Copy file name to clipboardExpand all lines: src/env.cc
+24Lines changed: 24 additions & 0 deletions
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) {
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+6Lines changed: 6 additions & 0 deletions
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

Collapse file

‎src/node.h‎

Copy file name to clipboardExpand all lines: src/node.h
+4-1Lines changed: 4 additions & 1 deletion
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

Collapse file

‎src/node_file.cc‎

Copy file name to clipboardExpand all lines: src/node_file.cc
+6Lines changed: 6 additions & 0 deletions
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
}
Collapse file

‎src/node_file.h‎

Copy file name to clipboardExpand all lines: src/node_file.h
+5-1Lines changed: 5 additions & 1 deletion
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.