lib,src: make StatWatcher a HandleWrap

Wrapping libuv handles is what `HandleWrap` is there for. This allows a decent reduction of state tracking machinery by moving active-ness tracking to JS, and removing all interaction with garbage collection. Refs: #21093 PR-URL: #21244 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

Author: addaleax

lib/internal/fs/watchers.js

@@ -11,41 +11,42 @@ const{
  getStatsFromBinding,
  validatePath
 }=require('internal/fs/utils');
+const{ defaultTriggerAsyncIdScope }=require('internal/async_hooks');
 const{ toNamespacedPath }=require('path');
 const{ validateUint32 }=require('internal/validators');
 const{ getPathFromURL }=require('internal/url');
 constutil=require('util');
 constassert=require('assert');
 
+constkOldStatus=Symbol('kOldStatus');
+constkUseBigint=Symbol('kUseBigint');
+constkOwner=Symbol('kOwner');
+
 functionemitStop(self){
 self.emit('stop');
 }
 
 functionStatWatcher(bigint){
 EventEmitter.call(this);
 
-this._handle=new_StatWatcher(bigint);
-
-// uv_fs_poll is a little more powerful than ev_stat but we curb it for
-// the sake of backwards compatibility
-letoldStatus=-1;
-
-this._handle.onchange=(newStatus,stats)=>{
-if(oldStatus===-1&&
-newStatus===-1&&
-stats[2/* new nlink */]===stats[16/* old nlink */])return;
-
-oldStatus=newStatus;
-this.emit('change',getStatsFromBinding(stats),
-getStatsFromBinding(stats,kFsStatsFieldsLength));
-};
-
-this._handle.onstop=()=>{
-process.nextTick(emitStop,this);
-};
+this._handle=null;
+this[kOldStatus]=-1;
+this[kUseBigint]=bigint;
 }
 util.inherits(StatWatcher,EventEmitter);
 
+functiononchange(newStatus,stats){
+constself=this[kOwner];
+if(self[kOldStatus]===-1&&
+newStatus===-1&&
+stats[2/* new nlink */]===stats[16/* old nlink */]){
+return;
+}
+
+self[kOldStatus]=newStatus;
+self.emit('change',getStatsFromBinding(stats),
+getStatsFromBinding(stats,kFsStatsFieldsLength));
+}
 
 // FIXME(joyeecheung): this method is not documented.
 // At the moment if filename is undefined, we
@@ -54,16 +55,23 @@ util.inherits(StatWatcher, EventEmitter);
 // on a valid filename and the wrap has been initialized
 // This method is a noop if the watcher has already been started.
 StatWatcher.prototype.start=function(filename,persistent,interval){
-assert(this._handleinstanceof_StatWatcher,'handle must be a StatWatcher');
-if(this._handle.isActive){
+if(this._handle!==null)
 return;
-}
+
+this._handle=new_StatWatcher(this[kUseBigint]);
+this._handle[kOwner]=this;
+this._handle.onchange=onchange;
+if(!persistent)
+this._handle.unref();
+
+// uv_fs_poll is a little more powerful than ev_stat but we curb it for
+// the sake of backwards compatibility
+this[kOldStatus]=-1;
 
 filename=getPathFromURL(filename);
 validatePath(filename,'filename');
 validateUint32(interval,'interval');
-consterr=this._handle.start(toNamespacedPath(filename),
-persistent,interval);
+consterr=this._handle.start(toNamespacedPath(filename),interval);
 if(err){
 consterror=errors.uvException({
 errno: err,
@@ -80,11 +88,15 @@ StatWatcher.prototype.start = function(filename, persistent, interval){
 // FSWatcher is .close()
 // This method is a noop if the watcher has not been started.
 StatWatcher.prototype.stop=function(){
-assert(this._handleinstanceof_StatWatcher,'handle must be a StatWatcher');
-if(!this._handle.isActive){
+if(this._handle===null)
 return;
-}
-this._handle.stop();
+
+defaultTriggerAsyncIdScope(this._handle.getAsyncId(),
+process.nextTick,
+emitStop,
+this);
+this._handle.close();
+this._handle=null;
 };
 
 

src/node_stat_watcher.cc

@@ -31,17 +31,12 @@
 namespacenode{
 
 using v8::Context;
-using v8::DontDelete;
-using v8::DontEnum;
 using v8::FunctionCallbackInfo;
 using v8::FunctionTemplate;
 using v8::HandleScope;
 using v8::Integer;
 using v8::Local;
 using v8::Object;
-using v8::PropertyAttribute;
-using v8::ReadOnly;
-using v8::Signature;
 using v8::String;
 using v8::Uint32;
 using v8::Value;
@@ -58,43 +53,32 @@ void StatWatcher::Initialize(Environment* env, Local<Object> target){
 
 AsyncWrap::AddWrapMethods(env, t);
  env->SetProtoMethod(t, "start", StatWatcher::Start);
- env->SetProtoMethod(t, "stop", StatWatcher::Stop);
-
- Local<FunctionTemplate> is_active_templ =
-FunctionTemplate::New(env->isolate(),
- IsActive,
- env->as_external(),
-Signature::New(env->isolate(), t));
- t->PrototypeTemplate()->SetAccessorProperty(
-FIXED_ONE_BYTE_STRING(env->isolate(), "isActive"),
- is_active_templ,
- Local<FunctionTemplate>(),
-static_cast<PropertyAttribute>(ReadOnly | DontDelete | DontEnum));
+ env->SetProtoMethod(t, "close", HandleWrap::Close);
+ env->SetProtoMethod(t, "ref", HandleWrap::Ref);
+ env->SetProtoMethod(t, "unref", HandleWrap::Unref);
+ env->SetProtoMethod(t, "hasRef", HandleWrap::HasRef);
 
  target->Set(statWatcherString, t->GetFunction());
 }
 
 
-StatWatcher::StatWatcher(Environment* env, Local<Object> wrap, bool use_bigint)
- : AsyncWrap(env, wrap, AsyncWrap::PROVIDER_STATWATCHER),
-watcher_(nullptr),
+StatWatcher::StatWatcher(Environment* env,
+ Local<Object> wrap,
+bool use_bigint)
+ : HandleWrap(env,
+ wrap,
+reinterpret_cast<uv_handle_t*>(&watcher_),
+ AsyncWrap::PROVIDER_STATWATCHER),
 use_bigint_(use_bigint){
-MakeWeak();
-}
-
-
-StatWatcher::~StatWatcher(){
-if (IsActive())
-Stop();
+CHECK_EQ(0, uv_fs_poll_init(env->event_loop(), &watcher_));
 }
 
 
 voidStatWatcher::Callback(uv_fs_poll_t* handle,
 int status,
 constuv_stat_t* prev,
 constuv_stat_t* curr){
- StatWatcher* wrap = static_cast<StatWatcher*>(handle->data);
-CHECK_EQ(wrap->watcher_, handle);
+ StatWatcher* wrap = ContainerOf(&StatWatcher::watcher_, handle);
  Environment* env = wrap->env();
  HandleScope handle_scope(env->isolate());
  Context::Scope context_scope(env->context());
@@ -118,78 +102,28 @@ void StatWatcher::New(const FunctionCallbackInfo<Value>& args){
 newStatWatcher(env, args.This(), args[0]->IsTrue());
 }
 
-boolStatWatcher::IsActive(){
-return watcher_ != nullptr;
-}
-
-voidStatWatcher::IsActive(const v8::FunctionCallbackInfo<v8::Value>& args){
- StatWatcher* wrap = Unwrap<StatWatcher>(args.This());
-CHECK_NOT_NULL(wrap);
- args.GetReturnValue().Set(wrap->IsActive());
-}
-
-// wrap.start(filename, persistent, interval)
+// wrap.start(filename, interval)
 voidStatWatcher::Start(const FunctionCallbackInfo<Value>& args){
-CHECK_EQ(args.Length(), 3);
+CHECK_EQ(args.Length(), 2);
 
- StatWatcher* wrap = Unwrap<StatWatcher>(args.Holder());
-CHECK_NOT_NULL(wrap);
-if (wrap->IsActive()){
-return;
- }
+ StatWatcher* wrap;
+ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
+CHECK(!uv_is_active(wrap->GetHandle()));
 
 constint argc = args.Length();
-CHECK_GE(argc, 3);
 
  node::Utf8Value path(args.GetIsolate(), args[0]);
 CHECK_NOT_NULL(*path);
 
-bool persistent = true;
-if (args[1]->IsFalse()){
- persistent = false;
- }
-
-CHECK(args[2]->IsUint32());
-constuint32_t interval = args[2].As<Uint32>()->Value();
-
- wrap->watcher_ = newuv_fs_poll_t();
-CHECK_EQ(0, uv_fs_poll_init(wrap->env()->event_loop(), wrap->watcher_));
- wrap->watcher_->data = static_cast<void*>(wrap);
-// Safe, uv_ref/uv_unref are idempotent.
-if (persistent)
-uv_ref(reinterpret_cast<uv_handle_t*>(wrap->watcher_));
-else
-uv_unref(reinterpret_cast<uv_handle_t*>(wrap->watcher_));
+CHECK(args[1]->IsUint32());
+constuint32_t interval = args[1].As<Uint32>()->Value();
 
 // Note that uv_fs_poll_start does not return ENOENT, we are handling
 // mostly memory errors here.
-constint err = uv_fs_poll_start(wrap->watcher_, Callback, *path, interval);
+constint err = uv_fs_poll_start(&wrap->watcher_, Callback, *path, interval);
 if (err != 0){
  args.GetReturnValue().Set(err);
  }
- wrap->ClearWeak();
 }
 
-
-voidStatWatcher::Stop(const FunctionCallbackInfo<Value>& args){
- StatWatcher* wrap = Unwrap<StatWatcher>(args.Holder());
-CHECK_NOT_NULL(wrap);
-if (!wrap->IsActive()){
-return;
- }
-
- Environment* env = wrap->env();
- Context::Scope context_scope(env->context());
- wrap->MakeCallback(env->onstop_string(), 0, nullptr);
- wrap->Stop();
-}
-
-
-voidStatWatcher::Stop(){
-env()->CloseHandle(watcher_, [](uv_fs_poll_t* handle){delete handle});
- watcher_ = nullptr;
-MakeWeak();
-}
-
-
 } // namespace node

src/node_stat_watcher.h

@@ -25,26 +25,24 @@
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
 #include"node.h"
-#include"async_wrap.h"
+#include"handle_wrap.h"
 #include"env.h"
 #include"uv.h"
 #include"v8.h"
 
 namespacenode{
 
-classStatWatcher : publicAsyncWrap{
+classStatWatcher : publicHandleWrap{
 public:
-~StatWatcher() override;
-
 staticvoidInitialize(Environment* env, v8::Local<v8::Object> target);
 
 protected:
-StatWatcher(Environment* env, v8::Local<v8::Object> wrap, bool use_bigint);
+StatWatcher(Environment* env,
+ v8::Local<v8::Object> wrap,
+bool use_bigint);
 
 staticvoidNew(const v8::FunctionCallbackInfo<v8::Value>& args);
 staticvoidStart(const v8::FunctionCallbackInfo<v8::Value>& args);
-staticvoidStop(const v8::FunctionCallbackInfo<v8::Value>& args);
-staticvoidIsActive(const v8::FunctionCallbackInfo<v8::Value>& args);
 
 size_tself_size() constoverride{returnsizeof(*this)}
 
@@ -53,10 +51,8 @@ class StatWatcher : public AsyncWrap{
 int status,
 constuv_stat_t* prev,
 constuv_stat_t* curr);
-voidStop();
-boolIsActive();
 
-uv_fs_poll_t* watcher_;
+uv_fs_poll_t watcher_;
 constbool use_bigint_;
 };
 

test/sequential/test-fs-watch.js

@@ -126,19 +126,3 @@ tmpdir.refresh();
 });
 oldhandle.close();// clean up
 }
-
-{
-letoldhandle;
-assert.throws(()=>{
-constw=fs.watchFile(__filename,
-{persistent: false},
-common.mustNotCall());
-oldhandle=w._handle;
-w._handle={stop: w._handle.stop};
-w.stop();
-},{
-message: 'handle must be a StatWatcher',
-code: 'ERR_ASSERTION'
-});
-oldhandle.stop();// clean up
-}