diff --git a/lib/internal/fs/watchers.js b/lib/internal/fs/watchers.js index 0863664110372e..f542a1eab1d177 100644 --- a/lib/internal/fs/watchers.js +++ b/lib/internal/fs/watchers.js @@ -2,7 +2,7 @@ const errors = require('internal/errors'); const { - kFsStatsFieldsLength, + kFsStatsFieldsNumber, StatWatcher: _StatWatcher } = process.binding('fs'); const { FSEvent } = internalBinding('fs_event_wrap'); @@ -48,7 +48,7 @@ function onchange(newStatus, stats) { self[kOldStatus] = newStatus; self.emit('change', getStatsFromBinding(stats), - getStatsFromBinding(stats, kFsStatsFieldsLength)); + getStatsFromBinding(stats, kFsStatsFieldsNumber)); } // FIXME(joyeecheung): this method is not documented. diff --git a/src/env.cc b/src/env.cc index 383df37aab88bd..79739e7d382991 100644 --- a/src/env.cc +++ b/src/env.cc @@ -169,8 +169,8 @@ Environment::Environment(IsolateData* isolate_data, trace_category_state_(isolate_, kTraceCategoryCount), stream_base_state_(isolate_, StreamBase::kNumStreamBaseStateFields), http_parser_buffer_(nullptr), - fs_stats_field_array_(isolate_, kFsStatsFieldsLength * 2), - fs_stats_field_bigint_array_(isolate_, kFsStatsFieldsLength * 2), + fs_stats_field_array_(isolate_, kFsStatsBufferLength), + fs_stats_field_bigint_array_(isolate_, kFsStatsBufferLength), context_(context->GetIsolate(), context) { // We'll be creating new objects so make sure we've entered the context. v8::HandleScope handle_scope(isolate()); diff --git a/src/env.h b/src/env.h index 36e4169b4b8978..af2dbd32a5a9c4 100644 --- a/src/env.h +++ b/src/env.h @@ -84,9 +84,12 @@ struct PackageConfig { // The number of items passed to push_values_to_array_function has diminishing // returns around 8. This should be used at all call sites using said function. -#ifndef NODE_PUSH_VAL_TO_ARRAY_MAX -#define NODE_PUSH_VAL_TO_ARRAY_MAX 8 -#endif +constexpr size_t NODE_PUSH_VAL_TO_ARRAY_MAX = 8; + +// Stat fields buffers contain twice the number of entries in an uv_stat_t +// because `fs.StatWatcher` needs room to store 2 `fs.Stats` instances. +constexpr size_t kFsStatsFieldsNumber = 14; +constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2; // PER_ISOLATE_* macros: We have a lot of per-isolate properties // and adding and maintaining their getters and setters by hand would be @@ -710,10 +713,6 @@ class Environment { inline AliasedBuffer* fs_stats_field_bigint_array(); - // stat fields contains twice the number of entries because `fs.StatWatcher` - // needs room to store data for *two* `fs.Stats` instances. - static const int kFsStatsFieldsLength = 14; - inline std::vector>& file_handle_read_wrap_freelist(); diff --git a/src/node_file.cc b/src/node_file.cc index 5b102269258e0b..920350f01a0297 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -426,7 +426,7 @@ void FSReqCallback::Reject(Local reject) { } void FSReqCallback::ResolveStat(const uv_stat_t* stat) { - Resolve(node::FillGlobalStatsArray(env(), stat, use_bigint())); + Resolve(FillGlobalStatsArray(env(), use_bigint(), stat)); } void FSReqCallback::Resolve(Local value) { @@ -949,8 +949,8 @@ static void Stat(const FunctionCallbackInfo& args) { return; // error info is in ctx } - Local arr = node::FillGlobalStatsArray(env, - static_cast(req_wrap_sync.req.ptr), use_bigint); + Local arr = FillGlobalStatsArray(env, use_bigint, + static_cast(req_wrap_sync.req.ptr)); args.GetReturnValue().Set(arr); } } @@ -980,8 +980,8 @@ static void LStat(const FunctionCallbackInfo& args) { return; // error info is in ctx } - Local arr = node::FillGlobalStatsArray(env, - static_cast(req_wrap_sync.req.ptr), use_bigint); + Local arr = FillGlobalStatsArray(env, use_bigint, + static_cast(req_wrap_sync.req.ptr)); args.GetReturnValue().Set(arr); } } @@ -1010,8 +1010,8 @@ static void FStat(const FunctionCallbackInfo& args) { return; // error info is in ctx } - Local arr = node::FillGlobalStatsArray(env, - static_cast(req_wrap_sync.req.ptr), use_bigint); + Local arr = FillGlobalStatsArray(env, use_bigint, + static_cast(req_wrap_sync.req.ptr)); args.GetReturnValue().Set(arr); } } @@ -2237,8 +2237,8 @@ void Initialize(Local target, env->SetMethod(target, "mkdtemp", Mkdtemp); target->Set(context, - FIXED_ONE_BYTE_STRING(isolate, "kFsStatsFieldsLength"), - Integer::New(isolate, env->kFsStatsFieldsLength)) + FIXED_ONE_BYTE_STRING(isolate, "kFsStatsFieldsNumber"), + Integer::New(isolate, kFsStatsFieldsNumber)) .FromJust(); target->Set(context, diff --git a/src/node_file.h b/src/node_file.h index 31242e1a1b1055..e2f8bdc55e6ad1 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -30,7 +30,7 @@ class FSContinuationData : public MemoryRetainer { uv_fs_t* req; int mode; - std::vector paths; + std::vector paths{}; void PushPath(std::string&& path) { paths.emplace_back(std::move(path)); @@ -148,6 +148,92 @@ class FSReqCallback : public FSReqBase { DISALLOW_COPY_AND_ASSIGN(FSReqCallback); }; +// Wordaround a GCC4.9 bug that C++14 N3652 was not implemented +// Refs: https://www.gnu.org/software/gcc/projects/cxx-status.html#cxx14 +// Refs: https://isocpp.org/files/papers/N3652.html +#if __cpp_constexpr < 201304 +# define constexpr inline +#endif + +template ::value>> +constexpr NativeT ToNative(uv_timespec_t ts) { + // This template has exactly two specializations below. + static_assert(std::is_arithmetic::value == false, "Not implemented"); + UNREACHABLE(); +} + +template <> +constexpr double ToNative(uv_timespec_t ts) { + // We need to do a static_cast since the original FS values are ulong. + /* NOLINTNEXTLINE(runtime/int) */ + const auto u_sec = static_cast(ts.tv_sec); + const double full_sec = u_sec * 1000.0; + /* NOLINTNEXTLINE(runtime/int) */ + const auto u_nsec = static_cast(ts.tv_nsec); + const double full_nsec = u_nsec / 1000'000.0; + return full_sec + full_nsec; +} + +template <> +constexpr uint64_t ToNative(uv_timespec_t ts) { + // We need to do a static_cast since the original FS values are ulong. + /* NOLINTNEXTLINE(runtime/int) */ + const auto u_sec = static_cast(ts.tv_sec); + const auto full_sec = static_cast(u_sec) * 1000UL; + /* NOLINTNEXTLINE(runtime/int) */ + const auto u_nsec = static_cast(ts.tv_nsec); + const auto full_nsec = static_cast(u_nsec) / 1000'000UL; + return full_sec + full_nsec; +} + +#undef constexpr // end N3652 bug workaround + +template +constexpr void FillStatsArray(AliasedBuffer* fields, + const uv_stat_t* s, const size_t offset = 0) { + fields->SetValue(offset + 0, s->st_dev); + fields->SetValue(offset + 1, s->st_mode); + fields->SetValue(offset + 2, s->st_nlink); + fields->SetValue(offset + 3, s->st_uid); + fields->SetValue(offset + 4, s->st_gid); + fields->SetValue(offset + 5, s->st_rdev); +#if defined(__POSIX__) + fields->SetValue(offset + 6, s->st_blksize); +#else + fields->SetValue(offset + 6, 0); +#endif + fields->SetValue(offset + 7, s->st_ino); + fields->SetValue(offset + 8, s->st_size); +#if defined(__POSIX__) + fields->SetValue(offset + 9, s->st_blocks); +#else + fields->SetValue(offset + 9, 0); +#endif +// Dates. + fields->SetValue(offset + 10, ToNative(s->st_atim)); + fields->SetValue(offset + 11, ToNative(s->st_mtim)); + fields->SetValue(offset + 12, ToNative(s->st_ctim)); + fields->SetValue(offset + 13, ToNative(s->st_birthtim)); +} + +inline Local FillGlobalStatsArray(Environment* env, + const bool use_bigint, + const uv_stat_t* s, + const bool second = false) { + const ptrdiff_t offset = second ? kFsStatsFieldsNumber : 0; + if (use_bigint) { + auto* const arr = env->fs_stats_field_bigint_array(); + FillStatsArray(arr, s, offset); + return arr->GetJSArray(); + } else { + auto* const arr = env->fs_stats_field_array(); + FillStatsArray(arr, s, offset); + return arr->GetJSArray(); + } +} + template class FSReqPromise : public FSReqBase { public: @@ -157,10 +243,11 @@ class FSReqPromise : public FSReqBase { ->NewInstance(env->context()).ToLocalChecked(), AsyncWrap::PROVIDER_FSREQPROMISE, use_bigint), - stats_field_array_(env->isolate(), env->kFsStatsFieldsLength) { - auto resolver = Promise::Resolver::New(env->context()).ToLocalChecked(); - object()->Set(env->context(), env->promise_string(), - resolver).FromJust(); + stats_field_array_(env->isolate(), kFsStatsFieldsNumber) { + const auto resolver = + Promise::Resolver::New(env->context()).ToLocalChecked(); + USE(object()->Set(env->context(), env->promise_string(), + resolver).FromJust()); } ~FSReqPromise() override { @@ -176,7 +263,7 @@ class FSReqPromise : public FSReqBase { object()->Get(env()->context(), env()->promise_string()).ToLocalChecked(); Local resolver = value.As(); - resolver->Reject(env()->context(), reject).FromJust(); + USE(resolver->Reject(env()->context(), reject).FromJust()); } void Resolve(Local value) override { @@ -187,11 +274,12 @@ class FSReqPromise : public FSReqBase { object()->Get(env()->context(), env()->promise_string()).ToLocalChecked(); Local resolver = val.As(); - resolver->Resolve(env()->context(), value).FromJust(); + USE(resolver->Resolve(env()->context(), value).FromJust()); } void ResolveStat(const uv_stat_t* stat) override { - Resolve(node::FillStatsArray(&stats_field_array_, stat)); + FillStatsArray(&stats_field_array_, stat); + Resolve(stats_field_array_.GetJSArray()); } void SetReturnValue(const FunctionCallbackInfo& args) override { @@ -210,10 +298,14 @@ class FSReqPromise : public FSReqBase { SET_MEMORY_INFO_NAME(FSReqPromise) SET_SELF_SIZE(FSReqPromise) + FSReqPromise(const FSReqPromise&) = delete; + FSReqPromise& operator=(const FSReqPromise&) = delete; + FSReqPromise(const FSReqPromise&&) = delete; + FSReqPromise& operator=(const FSReqPromise&&) = delete; + private: bool finished_ = false; AliasedBuffer stats_field_array_; - DISALLOW_COPY_AND_ASSIGN(FSReqPromise); }; class FSReqAfterScope { @@ -225,6 +317,11 @@ class FSReqAfterScope { void Reject(uv_fs_t* req); + FSReqAfterScope(const FSReqAfterScope&) = delete; + FSReqAfterScope& operator=(const FSReqAfterScope&) = delete; + FSReqAfterScope(const FSReqAfterScope&&) = delete; + FSReqAfterScope& operator=(const FSReqAfterScope&&) = delete; + private: FSReqBase* wrap_ = nullptr; uv_fs_t* req_ = nullptr; @@ -301,6 +398,11 @@ class FileHandle : public AsyncWrap, public StreamBase { SET_MEMORY_INFO_NAME(FileHandle) SET_SELF_SIZE(FileHandle) + FileHandle(const FileHandle&) = delete; + FileHandle& operator=(const FileHandle&) = delete; + FileHandle(const FileHandle&&) = delete; + FileHandle& operator=(const FileHandle&&) = delete; + private: // Synchronous close that emits a warning void Close(); @@ -319,7 +421,7 @@ class FileHandle : public AsyncWrap, public StreamBase { ref_.Reset(env->isolate(), ref); } - ~CloseReq() { + ~CloseReq() override { uv_fs_req_cleanup(req()); promise_.Reset(); ref_.Reset(); @@ -343,9 +445,14 @@ class FileHandle : public AsyncWrap, public StreamBase { return static_cast(ReqWrap::from_req(req)); } + CloseReq(const CloseReq&) = delete; + CloseReq& operator=(const CloseReq&) = delete; + CloseReq(const CloseReq&&) = delete; + CloseReq& operator=(const CloseReq&&) = delete; + private: - Persistent promise_; - Persistent ref_; + Persistent promise_{}; + Persistent ref_{}; }; // Asynchronous close @@ -359,9 +466,6 @@ class FileHandle : public AsyncWrap, public StreamBase { bool reading_ = false; std::unique_ptr current_read_ = nullptr; - - - DISALLOW_COPY_AND_ASSIGN(FileHandle); }; } // namespace fs diff --git a/src/node_internals.h b/src/node_internals.h index d0209fedc5b703..b3070198529b19 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -32,7 +32,6 @@ #include "uv.h" #include "v8.h" #include "tracing/trace_event.h" -#include "node_perf_common.h" #include "node_api.h" #include @@ -308,57 +307,6 @@ v8::Maybe ProcessEmitDeprecationWarning(Environment* env, const char* warning, const char* deprecation_code); -template -v8::Local FillStatsArray(AliasedBuffer* fields_ptr, - const uv_stat_t* s, int offset = 0) { - AliasedBuffer& fields = *fields_ptr; - fields[offset + 0] = s->st_dev; - fields[offset + 1] = s->st_mode; - fields[offset + 2] = s->st_nlink; - fields[offset + 3] = s->st_uid; - fields[offset + 4] = s->st_gid; - fields[offset + 5] = s->st_rdev; -#if defined(__POSIX__) - fields[offset + 6] = s->st_blksize; -#else - fields[offset + 6] = 0; -#endif - fields[offset + 7] = s->st_ino; - fields[offset + 8] = s->st_size; -#if defined(__POSIX__) - fields[offset + 9] = s->st_blocks; -#else - fields[offset + 9] = 0; -#endif -// Dates. -// NO-LINT because the fields are 'long' and we just want to cast to `unsigned` -#define X(idx, name) \ - /* NOLINTNEXTLINE(runtime/int) */ \ - fields[offset + idx] = ((unsigned long)(s->st_##name.tv_sec) * 1e3) + \ - /* NOLINTNEXTLINE(runtime/int) */ \ - ((unsigned long)(s->st_##name.tv_nsec) / 1e6); \ - - X(10, atim) - X(11, mtim) - X(12, ctim) - X(13, birthtim) -#undef X - - return fields_ptr->GetJSArray(); -} - -inline v8::Local FillGlobalStatsArray(Environment* env, - const uv_stat_t* s, - bool use_bigint = false, - int offset = 0) { - if (use_bigint) { - return node::FillStatsArray( - env->fs_stats_field_bigint_array(), s, offset); - } else { - return node::FillStatsArray(env->fs_stats_field_array(), s, offset); - } -} - void SetupBootstrapObject(Environment* env, v8::Local bootstrapper); void SetupProcessObject(Environment* env, diff --git a/src/node_stat_watcher.cc b/src/node_stat_watcher.cc index ca0927af662200..d586b52ede1ed5 100644 --- a/src/node_stat_watcher.cc +++ b/src/node_stat_watcher.cc @@ -22,8 +22,8 @@ #include "node_stat_watcher.h" #include "node_internals.h" #include "async_wrap-inl.h" -#include "env-inl.h" -#include "util-inl.h" +#include "env.h" +#include "node_file.h" #include #include @@ -80,15 +80,10 @@ void StatWatcher::Callback(uv_fs_poll_t* handle, HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); - Local arr = node::FillGlobalStatsArray(env, curr, - wrap->use_bigint_); - node::FillGlobalStatsArray(env, prev, wrap->use_bigint_, - env->kFsStatsFieldsLength); + Local arr = fs::FillGlobalStatsArray(env, wrap->use_bigint_, curr); + USE(fs::FillGlobalStatsArray(env, wrap->use_bigint_, prev, true)); - Local argv[2] { - Integer::New(env->isolate(), status), - arr - }; + Local argv[2] = { Integer::New(env->isolate(), status), arr }; wrap->MakeCallback(env->onchange_string(), arraysize(argv), argv); }