GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — src: show original file name in FileHandle GC close errors · nodejs/node@a31ad37

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

src
- dataqueue
  - queue.cc
- node_file.cc
- node_file.h
test/parallel
- test-fs-filehandle.js

Expand file treeCollapse file tree

Open diff view settings

Collapse file

`‎src/dataqueue/queue.cc‎`

Copy file name to clipboardExpand all lines: src/dataqueue/queue.cc

+1Lines changed: 1 addition & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -916,6 +916,7 @@ class FdEntry final : public EntryImpl {`
`916`	`916`	`fs::FileHandle::New(realm->GetBindingData<fs::BindingData>(),`
`917`	`917`	`file,`
`918`	`918`	`Local<Object>(),`
	`919`	`+ {},`
`919`	`920`	`entry->start_,`
`920`	`921`	`entry->end_ - entry->start_)),`
`921`	`922`	`entry);`

Collapse file

`‎src/node_file.cc‎`

Copy file name to clipboardExpand all lines: src/node_file.cc

+29-14Lines changed: 29 additions & 14 deletions

Original file line number	Diff line number	Diff line change
`@@ -222,9 +222,12 @@ void FSReqBase::MemoryInfo(MemoryTracker* tracker) const {`
`222`	`222`	`// collection if necessary. If that happens, a process warning will be`
`223`	`223`	`// emitted (or a fatal exception will occur if the fd cannot be closed.)`
`224`	`224`	`FileHandle::FileHandle(BindingData* binding_data,`
`225`		`- Local<Object> obj, int fd)`
	`225`	`+ Local<Object> obj,`
	`226`	`+ int fd,`
	`227`	`+ std::string original_name)`
`226`	`228`	`: AsyncWrap(binding_data->env(), obj, AsyncWrap::PROVIDER_FILEHANDLE),`
`227`	`229`	`StreamBase(env()),`
	`230`	`+ original_name_(std::move(original_name)),`
`228`	`231`	`fd_(fd),`
`229`	`232`	`binding_data_(binding_data) {`
`230`	`233`	`MakeWeak();`
`@@ -234,6 +237,7 @@ FileHandle::FileHandle(BindingData* binding_data,`
`234`	`237`	`FileHandle* FileHandle::New(BindingData* binding_data,`
`235`	`238`	`int fd,`
`236`	`239`	`Local<Object> obj,`
	`240`	`+ std::string original_name,`
`237`	`241`	`std::optional<int64_t> maybeOffset,`
`238`	`242`	`std::optional<int64_t> maybeLength) {`
`239`	`243`	`Environment* env = binding_data->env();`
`@@ -242,7 +246,7 @@ FileHandle* FileHandle::New(BindingData* binding_data,`
`242`	`246`	`.ToLocal(&obj)) {`
`243`	`247`	`return nullptr;`
`244`	`248`	`}`
`245`		`- auto handle = new FileHandle(binding_data, obj, fd);`
	`249`	`+ auto handle = new FileHandle(binding_data, obj, fd, original_name);`
`246`	`250`	`if (maybeOffset.has_value()) handle->read_offset_ = maybeOffset.value();`
`247`	`251`	`if (maybeLength.has_value()) handle->read_length_ = maybeLength.value();`
`248`	`252`	`return handle;`
`@@ -274,6 +278,7 @@ void FileHandle::New(const FunctionCallbackInfo<Value>& args) {`
`274`	`278`	`FileHandle::New(binding_data,`
`275`	`279`	`args[0].As<Int32>()->Value(),`
`276`	`280`	`args.This(),`
	`281`	`+ {},`
`277`	`282`	`maybeOffset,`
`278`	`283`	`maybeLength);`
`279`	`284`	`}`
`@@ -293,6 +298,7 @@ int FileHandle::DoWrite(WriteWrap* w,`
`293`	`298`
`294`	`299`	`void FileHandle::MemoryInfo(MemoryTracker* tracker) const {`
`295`	`300`	`tracker->TrackField("current_read", current_read_);`
	`301`	`+ tracker->TrackField("original_name", original_name_);`
`296`	`302`	`}`
`297`	`303`
`298`	`304`	`BaseObject::TransferMode FileHandle::GetTransferMode() const {`
`@@ -344,9 +350,13 @@ inline void FileHandle::Close() {`
`344`	`350`	`FS_SYNC_TRACE_END(close);`
`345`	`351`	`uv_fs_req_cleanup(&req);`
`346`	`352`
`347`		`- struct err_detail { int ret; int fd; };`
	`353`	`+ struct err_detail {`
	`354`	`+ int ret;`
	`355`	`+ int fd;`
	`356`	`+ std::string name;`
	`357`	`+ };`
`348`	`358`
`349`		`- err_detail detail { ret, fd_ };`
	`359`	`+ err_detail detail{ret, fd_, original_name_};`
`350`	`360`
`351`	`361`	`AfterClose();`
`352`	`362`
`@@ -362,25 +372,30 @@ inline void FileHandle::Close() {`
`362`	`372`	`// down the process is the only reasonable thing we can do here.`
`363`	`373`	`env()->SetImmediate([detail](Environment* env) {`
`364`	`374`	`HandleScope handle_scope(env->isolate());`
	`375`	`+ static constexpr std::string_view unknown_path = "<unknown path>";`
	`376`	`+ std::string_view filename =`
	`377`	`+ detail.name.empty() ? unknown_path : detail.name;`
`365`	`378`
`366`	`379`	`// If there was an error while trying to close the file descriptor,`
`367`	`380`	`// we will throw that instead.`
`368`	`381`	`if (detail.ret < 0) {`
`369`		`- char msg[70];`
`370`		`- snprintf(msg,`
`371`		`- arraysize(msg),`
`372`		`- "Closing file descriptor %d on garbage collection failed",`
`373`		`- detail.fd);`
	`382`	`+ auto formatted = SPrintF(`
	`383`	`+ "Closing file descriptor %d on garbage collection failed (%s)",`
	`384`	`+ detail.fd,`
	`385`	`+ filename);`
`374`	`386`	`HandleScope handle_scope(env->isolate());`
`375`		`- env->ThrowUVException(detail.ret, "close", msg);`
	`387`	`+ env->ThrowUVException(detail.ret, "close", formatted.c_str());`
`376`	`388`	`return;`
`377`	`389`	`}`
`378`	`390`
`379`	`391`	`THROW_ERR_INVALID_STATE(`
`380`	`392`	`env,`
`381`	`393`	`"A FileHandle object was closed during garbage collection. "`
`382`	`394`	`"This used to be allowed with a deprecation warning but is now "`
`383`		`- "considered an error. Please close FileHandle objects explicitly.");`
	`395`	`+ "considered an error. Please close FileHandle objects explicitly. "`
	`396`	`+ "File descriptor: %d (%s)",`
	`397`	`+ detail.fd,`
	`398`	`+ filename);`
`384`	`399`	`});`
`385`	`400`	`}`
`386`	`401`
`@@ -824,8 +839,8 @@ void AfterOpenFileHandle(uv_fs_t* req) {`
`824`	`839`	`FS_ASYNC_TRACE_END1(`
`825`	`840`	`req->fs_type, req_wrap, "result", static_cast<int>(req->result))`
`826`	`841`	`if (after.Proceed()) {`
`827`		`- FileHandle* fd = FileHandle::New(req_wrap->binding_data(),`
`828`		`- static_cast<int>(req->result));`
	`842`	`+ FileHandle* fd = FileHandle::New(`
	`843`	`+ req_wrap->binding_data(), static_cast<int>(req->result), {}, req->path);`
`829`	`844`	`if (fd == nullptr) return;`
`830`	`845`	`req_wrap->Resolve(fd->object());`
`831`	`846`	`}`
`@@ -2222,7 +2237,7 @@ static void OpenFileHandle(const FunctionCallbackInfo<Value>& args) {`
`2222`	`2237`	`if (result < 0) {`
`2223`	`2238`	`return; // syscall failed, no need to continue, error info is in ctx`
`2224`	`2239`	`}`
`2225`		`- FileHandle* fd = FileHandle::New(binding_data, result);`
	`2240`	`+ FileHandle* fd = FileHandle::New(binding_data, result, {}, path.ToString());`
`2226`	`2241`	`if (fd == nullptr) return;`
`2227`	`2242`	`args.GetReturnValue().Set(fd->object());`
`2228`	`2243`	`}`

Collapse file

`‎src/node_file.h‎`

Copy file name to clipboardExpand all lines: src/node_file.h

+6-1Lines changed: 6 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -329,6 +329,7 @@ class FileHandle final : public AsyncWrap, public StreamBase {`
`329`	`329`	`static FileHandle* New(BindingData* binding_data,`
`330`	`330`	`int fd,`
`331`	`331`	`v8::Local<v8::Object> obj = v8::Local<v8::Object>(),`
	`332`	`+ std::string original_name = {},`
`332`	`333`	`std::optional<int64_t> maybeOffset = std::nullopt,`
`333`	`334`	`std::optional<int64_t> maybeLength = std::nullopt);`
`334`	`335`	`~FileHandle() override;`
`@@ -395,7 +396,10 @@ class FileHandle final : public AsyncWrap, public StreamBase {`
`395`	`396`	`int fd_;`
`396`	`397`	`};`
`397`	`398`
`398`		`- FileHandle(BindingData* binding_data, v8::Local<v8::Object> obj, int fd);`
	`399`	`+ FileHandle(BindingData* binding_data,`
	`400`	`+ v8::Local<v8::Object> obj,`
	`401`	`+ int fd,`
	`402`	`+ std::string original_name);`
`399`	`403`
`400`	`404`	`// Synchronous close that emits a warning`
`401`	`405`	`void Close();`
`@@ -437,6 +441,7 @@ class FileHandle final : public AsyncWrap, public StreamBase {`
`437`	`441`	`// Asynchronous close`
`438`	`442`	`v8::MaybeLocal<v8::Promise> ClosePromise();`
`439`	`443`
	`444`	`+ std::string original_name_;`
`440`	`445`	`int fd_;`
`441`	`446`	`bool closing_ = false;`
`442`	`447`	`bool closed_ = false;`

Collapse file

`‎test/parallel/test-fs-filehandle.js‎`

Copy file name to clipboardExpand all lines: test/parallel/test-fs-filehandle.js

+4-1Lines changed: 4 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -8,17 +8,20 @@ const { internalBinding } = require('internal/test/binding');`
`8`	`8`	`const fs = internalBinding('fs');`
`9`	`9`	`const { stringToFlags } = require('internal/fs/utils');`
`10`	`10`
	`11`	`+const filepath = path.toNamespacedPath(__filename);`
	`12`	`+`
`11`	`13`	`// Verifies that the FileHandle object is garbage collected and that an`
`12`	`14`	`// error is thrown if it is not closed.`
`13`	`15`	`process.on('uncaughtException', common.mustCall((err) => {`
`14`	`16`	`assert.strictEqual(err.code, 'ERR_INVALID_STATE');`
`15`	`17`	`assert.match(err.message, /^A FileHandle object was closed during/);`
	`18`	`+ assert.match(err.message, new RegExp(RegExp.escape(filepath)));`
`16`	`19`	`}));`
`17`	`20`
`18`	`21`
`19`	`22`	`{`
`20`	`23`	`const ctx = {};`
`21`		`- fs.openFileHandle(path.toNamespacedPath(__filename),`
	`24`	`+ fs.openFileHandle(filepath,`
`22`	`25`	`stringToFlags('r'), 0o666, undefined, ctx);`
`23`	`26`	`assert.strictEqual(ctx.errno, undefined);`
`24`	`27`	`}`

0 commit comments

Comments

0 (0)

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.