src: move fs state out of Environment

Moves state that is specific to the `fs` binding into the `fs` binding implementation as a cleanup. PR-URL: #32538 Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Apr 6, 2020 · f4c2dff · f4c2dff
1 parent 1216e8f
commit f4c2dff
Show file tree

Hide file tree

Showing 9 changed files with 175 additions and 143 deletions.
diff --git a/src/env-inl.h b/src/env-inl.h
@@ -580,19 +580,6 @@ inline double Environment::get_default_trigger_async_id() {
   return default_trigger_async_id;
 }
 
-inline AliasedFloat64Array* Environment::fs_stats_field_array() {
-  return &fs_stats_field_array_;
-}
-
-inline AliasedBigUint64Array* Environment::fs_stats_field_bigint_array() {
-  return &fs_stats_field_bigint_array_;
-}
-
-inline std::vector<std::unique_ptr<fs::FileHandleReadWrap>>&
-Environment::file_handle_read_wrap_freelist() {
-  return file_handle_read_wrap_freelist_;
-}
-
 inline std::shared_ptr<EnvironmentOptions> Environment::options() {
   return options_;
 }

diff --git a/src/env.cc b/src/env.cc
@@ -1,18 +1,19 @@
 #include "env.h"
 
 #include "async_wrap.h"
+#include "base_object-inl.h"
 #include "debug_utils-inl.h"
 #include "memory_tracker-inl.h"
 #include "node_buffer.h"
 #include "node_context_data.h"
 #include "node_errors.h"
-#include "node_file.h"
 #include "node_internals.h"
 #include "node_options-inl.h"
 #include "node_process.h"
 #include "node_v8_platform-inl.h"
 #include "node_worker.h"
 #include "req_wrap-inl.h"
+#include "stream_base.h"
 #include "tracing/agent.h"
 #include "tracing/traced_value.h"
 #include "util-inl.h"
@@ -341,8 +342,6 @@ Environment::Environment(IsolateData* isolate_data,
       flags_(flags),
       thread_id_(thread_id.id == static_cast<uint64_t>(-1) ?
           AllocateEnvironmentThreadId().id : thread_id.id),
-      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.
   HandleScope handle_scope(isolate());
@@ -444,10 +443,6 @@ Environment::~Environment() {
   isolate()->GetHeapProfiler()->RemoveBuildEmbedderGraphCallback(
       BuildEmbedderGraph, this);
 
-  // Make sure there are no re-used libuv wrapper objects.
-  // CleanupHandles() should have removed all of them.
-  CHECK(file_handle_read_wrap_freelist_.empty());
-
   HandleScope handle_scope(isolate());
 
 #if HAVE_INSPECTOR
@@ -613,8 +608,6 @@ void Environment::CleanupHandles() {
          !handle_wrap_queue_.IsEmpty()) {
     uv_run(event_loop(), UV_RUN_ONCE);
   }
-
-  file_handle_read_wrap_freelist_.clear();
 }
 
 void Environment::StartProfilerIdleNotifier() {
@@ -1096,9 +1089,6 @@ void Environment::MemoryInfo(MemoryTracker* tracker) const {
   tracker->TrackField("should_abort_on_uncaught_toggle",
                       should_abort_on_uncaught_toggle_);
   tracker->TrackField("stream_base_state", stream_base_state_);
-  tracker->TrackField("fs_stats_field_array", fs_stats_field_array_);
-  tracker->TrackField("fs_stats_field_bigint_array",
-                      fs_stats_field_bigint_array_);
   tracker->TrackField("cleanup_hooks", cleanup_hooks_);
   tracker->TrackField("async_hooks", async_hooks_);
   tracker->TrackField("immediate_info", immediate_info_);

diff --git a/src/env.h b/src/env.h
@@ -56,10 +56,6 @@ class ContextifyScript;
 class CompiledFnEntry;
 }
 
-namespace fs {
-class FileHandleReadWrap;
-}
-
 namespace performance {
 class PerformanceState;
 }
@@ -1009,12 +1005,6 @@ class Environment : public MemoryRetainer {
 
   EnabledDebugList* enabled_debug_list() { return &enabled_debug_list_; }
 
-  inline AliasedFloat64Array* fs_stats_field_array();
-  inline AliasedBigUint64Array* fs_stats_field_bigint_array();
-
-  inline std::vector<std::unique_ptr<fs::FileHandleReadWrap>>&
-      file_handle_read_wrap_freelist();
-
   inline performance::PerformanceState* performance_state();
   inline std::unordered_map<std::string, uint64_t>* performance_marks();
 
@@ -1365,12 +1355,6 @@ class Environment : public MemoryRetainer {
 
   EnabledDebugList enabled_debug_list_;
 
-  AliasedFloat64Array fs_stats_field_array_;
-  AliasedBigUint64Array fs_stats_field_bigint_array_;
-
-  std::vector<std::unique_ptr<fs::FileHandleReadWrap>>
-      file_handle_read_wrap_freelist_;
-
   std::list<node_module> extra_linked_bindings_;
   Mutex extra_linked_bindings_mutex_;
 

diff --git a/src/node_dir.cc b/src/node_dir.cc
@@ -152,7 +152,7 @@ void DirHandle::Close(const FunctionCallbackInfo<Value>& args) {
   dir->closing_ = false;
   dir->closed_ = true;
 
-  FSReqBase* req_wrap_async = GetReqWrap(env, args[0]);
+  FSReqBase* req_wrap_async = GetReqWrap(args, 0);
   if (req_wrap_async != nullptr) {  // close(req)
     AsyncCall(env, req_wrap_async, args, "closedir", UTF8, AfterClose,
               uv_fs_closedir, dir->dir());
@@ -252,7 +252,7 @@ void DirHandle::Read(const FunctionCallbackInfo<Value>& args) {
     dir->dir_->dirents = dir->dirents_.data();
   }
 
-  FSReqBase* req_wrap_async = GetReqWrap(env, args[2]);
+  FSReqBase* req_wrap_async = GetReqWrap(args, 2);
   if (req_wrap_async != nullptr) {  // dir.read(encoding, bufferSize, req)
     AsyncCall(env, req_wrap_async, args, "readdir", encoding,
               AfterDirRead, uv_fs_readdir, dir->dir());
@@ -320,7 +320,7 @@ static void OpenDir(const FunctionCallbackInfo<Value>& args) {
 
   const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8);
 
-  FSReqBase* req_wrap_async = GetReqWrap(env, args[2]);
+  FSReqBase* req_wrap_async = GetReqWrap(args, 2);
   if (req_wrap_async != nullptr) {  // openDir(path, encoding, req)
     AsyncCall(env, req_wrap_async, args, "opendir", encoding, AfterOpenDir,
               uv_fs_opendir, *path);

diff --git a/src/node_file-inl.h b/src/node_file-inl.h
@@ -39,11 +39,13 @@ void FSContinuationData::Done(int result) {
   done_cb_(req_);
 }
 
-FSReqBase::FSReqBase(Environment* env,
-          v8::Local<v8::Object> req,
-          AsyncWrap::ProviderType type,
-          bool use_bigint)
-  : ReqWrap(env, req, type), use_bigint_(use_bigint) {
+FSReqBase::FSReqBase(BindingData* binding_data,
+                     v8::Local<v8::Object> req,
+                     AsyncWrap::ProviderType type,
+                     bool use_bigint)
+  : ReqWrap(binding_data->env(), req, type),
+    use_bigint_(use_bigint),
+    binding_data_(binding_data) {
 }
 
 void FSReqBase::Init(const char* syscall,
@@ -72,9 +74,13 @@ FSReqBase::Init(const char* syscall, size_t len, enum encoding encoding) {
   return buffer_;
 }
 
-FSReqCallback::FSReqCallback(Environment* env,
-                             v8::Local<v8::Object> req, bool use_bigint)
-  : FSReqBase(env, req, AsyncWrap::PROVIDER_FSREQCALLBACK, use_bigint) {}
+FSReqCallback::FSReqCallback(BindingData* binding_data,
+                             v8::Local<v8::Object> req,
+                             bool use_bigint)
+  : FSReqBase(binding_data,
+              req,
+              AsyncWrap::PROVIDER_FSREQCALLBACK,
+              use_bigint) {}
 
 template <typename NativeT, typename V8T>
 void FillStatsArray(AliasedBufferBase<NativeT, V8T>* fields,
@@ -112,26 +118,28 @@ void FillStatsArray(AliasedBufferBase<NativeT, V8T>* fields,
 #undef SET_FIELD_WITH_STAT
 }
 
-v8::Local<v8::Value> FillGlobalStatsArray(Environment* env,
+v8::Local<v8::Value> FillGlobalStatsArray(BindingData* binding_data,
                                           const bool use_bigint,
                                           const uv_stat_t* s,
                                           const bool second) {
   const ptrdiff_t offset =
       second ? static_cast<ptrdiff_t>(FsStatsOffset::kFsStatsFieldsNumber) : 0;
   if (use_bigint) {
-    auto* const arr = env->fs_stats_field_bigint_array();
+    auto* const arr = &binding_data->stats_field_bigint_array;
     FillStatsArray(arr, s, offset);
     return arr->GetJSArray();
   } else {
-    auto* const arr = env->fs_stats_field_array();
+    auto* const arr = &binding_data->stats_field_array;
     FillStatsArray(arr, s, offset);
     return arr->GetJSArray();
   }
 }
 
 template <typename AliasedBufferT>
 FSReqPromise<AliasedBufferT>*
-FSReqPromise<AliasedBufferT>::New(Environment* env, bool use_bigint) {
+FSReqPromise<AliasedBufferT>::New(BindingData* binding_data,
+                                  bool use_bigint) {
+  Environment* env = binding_data->env();
   v8::Local<v8::Object> obj;
   if (!env->fsreqpromise_constructor_template()
            ->NewInstance(env->context())
@@ -143,7 +151,7 @@ FSReqPromise<AliasedBufferT>::New(Environment* env, bool use_bigint) {
       obj->Set(env->context(), env->promise_string(), resolver).IsNothing()) {
     return nullptr;
   }
-  return new FSReqPromise(env, obj, use_bigint);
+  return new FSReqPromise(binding_data, obj, use_bigint);
 }
 
 template <typename AliasedBufferT>
@@ -154,12 +162,15 @@ FSReqPromise<AliasedBufferT>::~FSReqPromise() {
 
 template <typename AliasedBufferT>
 FSReqPromise<AliasedBufferT>::FSReqPromise(
-    Environment* env,
+    BindingData* binding_data,
     v8::Local<v8::Object> obj,
     bool use_bigint)
-  : FSReqBase(env, obj, AsyncWrap::PROVIDER_FSREQPROMISE, use_bigint),
+  : FSReqBase(binding_data,
+              obj,
+              AsyncWrap::PROVIDER_FSREQPROMISE,
+              use_bigint),
     stats_field_array_(
-        env->isolate(),
+        env()->isolate(),
         static_cast<size_t>(FsStatsOffset::kFsStatsFieldsNumber)) {}
 
 template <typename AliasedBufferT>
@@ -208,15 +219,21 @@ void FSReqPromise<AliasedBufferT>::MemoryInfo(MemoryTracker* tracker) const {
   tracker->TrackField("stats_field_array", stats_field_array_);
 }
 
-FSReqBase* GetReqWrap(Environment* env, v8::Local<v8::Value> value,
+FSReqBase* GetReqWrap(const v8::FunctionCallbackInfo<v8::Value>& args,
+                      int index,
                       bool use_bigint) {
+  v8::Local<v8::Value> value = args[index];
   if (value->IsObject()) {
     return Unwrap<FSReqBase>(value.As<v8::Object>());
-  } else if (value->StrictEquals(env->fs_use_promises_symbol())) {
+  }
+
+  BindingData* binding_data = Unwrap<BindingData>(args.Data());
+  Environment* env = binding_data->env();
+  if (value->StrictEquals(env->fs_use_promises_symbol())) {
     if (use_bigint) {
-      return FSReqPromise<AliasedBigUint64Array>::New(env, use_bigint);
+      return FSReqPromise<AliasedBigUint64Array>::New(binding_data, use_bigint);
     } else {
-      return FSReqPromise<AliasedFloat64Array>::New(env, use_bigint);
+      return FSReqPromise<AliasedFloat64Array>::New(binding_data, use_bigint);
     }
   }
   return nullptr;