diff --git a/src/node_file.cc b/src/node_file.cc index c9bbea1ee5f621..c8cdc7959153f5 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -94,6 +94,7 @@ using v8::MaybeLocal; using v8::Number; using v8::Object; using v8::String; +using v8::Undefined; using v8::Value; #ifndef MIN @@ -102,32 +103,16 @@ using v8::Value; #define GET_OFFSET(a) ((a)->IsNumber() ? (a)->IntegerValue() : -1) -FSReqWrap* FSReqWrap::New(Environment* env, - Local req, - const char* syscall, - const char* data, - enum encoding encoding, - Ownership ownership) { - const bool copy = (data != nullptr && ownership == COPY); - const size_t size = copy ? 1 + strlen(data) : 0; - FSReqWrap* that; - char* const storage = new char[sizeof(*that) + size]; - that = new(storage) FSReqWrap(env, req, syscall, data, encoding); - if (copy) - that->data_ = static_cast(memcpy(that->inline_data(), data, size)); - return that; +void FSReqWrap::Reject(Local reject) { + MakeCallback(env()->oncomplete_string(), 1, &reject); } - -void FSReqWrap::Dispose() { - this->~FSReqWrap(); - delete[] reinterpret_cast(this); +void FSReqWrap::FillStatsArray(const uv_stat_t* stat) { + node::FillStatsArray(env()->fs_stats_field_array(), stat); } - -void FSReqWrap::Reject(Local reject) { - Local argv[1] { reject }; - MakeCallback(env()->oncomplete_string(), arraysize(argv), argv); +void FSReqWrap::ResolveStat() { + Resolve(Undefined(env()->isolate())); } void FSReqWrap::Resolve(Local value) { @@ -138,9 +123,26 @@ void FSReqWrap::Resolve(Local value) { MakeCallback(env()->oncomplete_string(), arraysize(argv), argv); } +void FSReqWrap::Init(const char* syscall, + const char* data, + size_t len, + enum encoding encoding) { + syscall_ = syscall; + encoding_ = encoding; + + if (data != nullptr) { + CHECK_EQ(data_, nullptr); + buffer_.AllocateSufficientStorage(len + 1); + buffer_.SetLengthAndZeroTerminate(len); + memcpy(*buffer_, data, len); + data_ = *buffer_; + } +} + void NewFSReqWrap(const FunctionCallbackInfo& args) { CHECK(args.IsConstructCall()); - ClearWrap(args.This()); + Environment* env = Environment::GetCurrent(args.GetIsolate()); + new FSReqWrap(env, args.This()); } @@ -150,12 +152,11 @@ FSReqAfterScope::FSReqAfterScope(FSReqWrap* wrap, uv_fs_t* req) handle_scope_(wrap->env()->isolate()), context_scope_(wrap->env()->context()) { CHECK_EQ(wrap_->req(), req); - wrap_->ReleaseEarly(); // Free memory that's no longer used now. } FSReqAfterScope::~FSReqAfterScope() { uv_fs_req_cleanup(wrap_->req()); - wrap_->Dispose(); + delete wrap_; } // TODO(joyeecheung): create a normal context object, and @@ -195,12 +196,10 @@ void AfterNoArgs(uv_fs_t* req) { void AfterStat(uv_fs_t* req) { FSReqWrap* req_wrap = static_cast(req->data); FSReqAfterScope after(req_wrap, req); - Environment* env = req_wrap->env(); if (after.Proceed()) { - FillStatsArray(env->fs_stats_field_array(), - static_cast(req->ptr)); - req_wrap->Resolve(Undefined(req_wrap->env()->isolate())); + req_wrap->FillStatsArray(&req->statbuf); + req_wrap->ResolveStat(); } } @@ -222,7 +221,7 @@ void AfterStringPath(uv_fs_t* req) { if (after.Proceed()) { link = StringBytes::Encode(req_wrap->env()->isolate(), static_cast(req->path), - req_wrap->encoding_, + req_wrap->encoding(), &error); if (link.IsEmpty()) req_wrap->Reject(error); @@ -241,7 +240,7 @@ void AfterStringPtr(uv_fs_t* req) { if (after.Proceed()) { link = StringBytes::Encode(req_wrap->env()->isolate(), static_cast(req->ptr), - req_wrap->encoding_, + req_wrap->encoding(), &error); if (link.IsEmpty()) req_wrap->Reject(error); @@ -278,7 +277,7 @@ void AfterScanDir(uv_fs_t* req) { MaybeLocal filename = StringBytes::Encode(env->isolate(), ent.name, - req_wrap->encoding_, + req_wrap->encoding(), &error); if (filename.IsEmpty()) return req_wrap->Reject(error); @@ -317,12 +316,12 @@ class fs_req_wrap { template inline FSReqWrap* AsyncDestCall(Environment* env, const FunctionCallbackInfo& args, - const char* syscall, const char* dest, - enum encoding enc, FSReqWrap::Ownership ownership, - uv_fs_cb after, Func fn, Args... fn_args) { + const char* syscall, const char* dest, size_t len, + enum encoding enc, uv_fs_cb after, Func fn, Args... fn_args) { Local req = args[args.Length() - 1].As(); - FSReqWrap* req_wrap = FSReqWrap::New(env, req, - syscall, dest, enc, ownership); + FSReqWrap* req_wrap = Unwrap(req); + CHECK_NE(req_wrap, nullptr); + req_wrap->Init(syscall, dest, len, enc); int err = fn(env->event_loop(), req_wrap->req(), fn_args..., after); req_wrap->Dispatched(); if (err < 0) { @@ -339,38 +338,16 @@ inline FSReqWrap* AsyncDestCall(Environment* env, return req_wrap; } -// Defaults to COPY ownership. -template -inline FSReqWrap* AsyncDestCall(Environment* env, - const FunctionCallbackInfo& args, - const char* syscall, const char* dest, enum encoding enc, - uv_fs_cb after, Func fn, Args... fn_args) { - return AsyncDestCall(env, args, - syscall, dest, enc, FSReqWrap::COPY, - after, fn, fn_args...); -} - template inline FSReqWrap* AsyncCall(Environment* env, const FunctionCallbackInfo& args, - const char* syscall, enum encoding enc, FSReqWrap::Ownership ownership, + const char* syscall, enum encoding enc, uv_fs_cb after, Func fn, Args... fn_args) { return AsyncDestCall(env, args, - syscall, nullptr, enc, ownership, + syscall, nullptr, 0, enc, after, fn, fn_args...); } -// Defaults to COPY ownership. -template -inline FSReqWrap* AsyncCall(Environment* env, - const FunctionCallbackInfo& args, - const char* syscall, enum encoding enc, - uv_fs_cb after, Func fn, Args... fn_args) { - return AsyncCall(env, args, - syscall, enc, FSReqWrap::COPY, - after, fn, fn_args...); -} - // Template counterpart of SYNC_CALL, except that it only puts // the error number and the syscall in the context instead of // creating an error in the C++ land. @@ -623,8 +600,8 @@ static void Symlink(const FunctionCallbackInfo& args) { if (args[3]->IsObject()) { // symlink(target, path, flags, req) CHECK_EQ(args.Length(), 4); - AsyncDestCall(env, args, "symlink", *path, UTF8, AfterNoArgs, - uv_fs_symlink, *target, *path, flags); + AsyncDestCall(env, args, "symlink", *path, path.length(), UTF8, + AfterNoArgs, uv_fs_symlink, *target, *path, flags); } else { // symlink(target, path, flags) SYNC_DEST_CALL(symlink, *target, *path, *target, *path, flags) } @@ -643,8 +620,8 @@ static void Link(const FunctionCallbackInfo& args) { if (args[2]->IsObject()) { // link(src, dest, req) CHECK_EQ(args.Length(), 3); - AsyncDestCall(env, args, "link", *dest, UTF8, AfterNoArgs, - uv_fs_link, *src, *dest); + AsyncDestCall(env, args, "link", *dest, dest.length(), UTF8, + AfterNoArgs, uv_fs_link, *src, *dest); } else { // link(src, dest) SYNC_DEST_CALL(link, *src, *dest, *src, *dest) } @@ -695,8 +672,8 @@ static void Rename(const FunctionCallbackInfo& args) { if (args[2]->IsObject()) { CHECK_EQ(args.Length(), 3); - AsyncDestCall(env, args, "rename", *new_path, UTF8, AfterNoArgs, - uv_fs_rename, *old_path, *new_path); + AsyncDestCall(env, args, "rename", *new_path, new_path.length(), + UTF8, AfterNoArgs, uv_fs_rename, *old_path, *new_path); } else { SYNC_DEST_CALL(rename, *old_path, *new_path, *old_path, *new_path) } @@ -1041,7 +1018,6 @@ static void WriteString(const FunctionCallbackInfo& args) { char* buf = nullptr; int64_t pos; size_t len; - FSReqWrap::Ownership ownership = FSReqWrap::COPY; // will assign buf and len if string was external if (!StringBytes::GetExternalParts(string, @@ -1053,7 +1029,6 @@ static void WriteString(const FunctionCallbackInfo& args) { // StorageSize may return too large a char, so correct the actual length // by the write size len = StringBytes::Write(env->isolate(), buf, len, args[1], enc); - ownership = FSReqWrap::MOVE; } pos = GET_OFFSET(args[2]); @@ -1061,8 +1036,7 @@ static void WriteString(const FunctionCallbackInfo& args) { if (args[4]->IsObject()) { CHECK_EQ(args.Length(), 5); - AsyncCall(env, args, - "write", UTF8, ownership, AfterInteger, + AsyncCall(env, args, "write", UTF8, AfterInteger, uv_fs_write, fd, &uvbuf, 1, pos); } else { // SYNC_CALL returns on error. Make sure to always free the memory. @@ -1071,7 +1045,7 @@ static void WriteString(const FunctionCallbackInfo& args) { inline ~Delete() { delete[] pointer_; } char* const pointer_; }; - Delete delete_on_return(ownership == FSReqWrap::MOVE ? buf : nullptr); + Delete delete_on_return(buf); SYNC_CALL(write, nullptr, fd, &uvbuf, 1, pos) return args.GetReturnValue().Set(SYNC_RESULT); } @@ -1373,7 +1347,7 @@ void InitFs(Local target, Local wrapString = FIXED_ONE_BYTE_STRING(env->isolate(), "FSReqWrap"); fst->SetClassName(wrapString); - target->Set(wrapString, fst->GetFunction()); + target->Set(context, wrapString, fst->GetFunction()).FromJust(); } } // namespace fs diff --git a/src/node_file.h b/src/node_file.h index db85451b67a9df..878d02c8ba4989 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -17,60 +17,39 @@ using v8::Value; namespace fs { -class FSReqWrap: public ReqWrap { +class FSReqWrap : public ReqWrap { public: - enum Ownership { COPY, MOVE }; + FSReqWrap(Environment* env, Local req) + : ReqWrap(env, req, AsyncWrap::PROVIDER_FSREQWRAP) { + Wrap(object(), this); + } - inline static FSReqWrap* New(Environment* env, - Local req, - const char* syscall, - const char* data = nullptr, - enum encoding encoding = UTF8, - Ownership ownership = COPY); + virtual ~FSReqWrap() { + ClearWrap(object()); + } - inline void Dispose(); + void Init(const char* syscall, + const char* data = nullptr, + size_t len = 0, + enum encoding encoding = UTF8); + virtual void FillStatsArray(const uv_stat_t* stat); virtual void Reject(Local reject); virtual void Resolve(Local value); - - void ReleaseEarly() { - if (data_ != inline_data()) { - delete[] data_; - data_ = nullptr; - } - } + virtual void ResolveStat(); const char* syscall() const { return syscall_; } const char* data() const { return data_; } - const enum encoding encoding_; + enum encoding encoding() const { return encoding_; } size_t self_size() const override { return sizeof(*this); } - protected: - FSReqWrap(Environment* env, - Local req, - const char* syscall, - const char* data, - enum encoding encoding) - : ReqWrap(env, req, AsyncWrap::PROVIDER_FSREQWRAP), - encoding_(encoding), - syscall_(syscall), - data_(data) { - Wrap(object(), this); - } - - virtual ~FSReqWrap() { - ReleaseEarly(); - ClearWrap(object()); - } - - void* operator new(size_t size) = delete; - void* operator new(size_t size, char* storage) { return storage; } - char* inline_data() { return reinterpret_cast(this + 1); } - private: + enum encoding encoding_ = UTF8; const char* syscall_; - const char* data_; + + const char* data_ = nullptr; + MaybeStackBuffer buffer_; DISALLOW_COPY_AND_ASSIGN(FSReqWrap); }; diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js index 58d6b7746985c5..f5b6843ef92fb9 100644 --- a/test/sequential/test-async-wrap-getasyncid.js +++ b/test/sequential/test-async-wrap-getasyncid.js @@ -112,9 +112,8 @@ if (common.hasCrypto) { // eslint-disable-line crypto-check const req = new FSReqWrap(); req.oncomplete = () => { }; - testUninitialized(req, 'FSReqWrap'); - binding.access(path.toNamespacedPath('../'), fs.F_OK, req); testInitialized(req, 'FSReqWrap'); + binding.access(path.toNamespacedPath('../'), fs.F_OK, req); const StatWatcher = binding.StatWatcher; testInitialized(new StatWatcher(), 'StatWatcher');