diff --git a/Makefile b/Makefile index a4f68303..a29287b4 100644 --- a/Makefile +++ b/Makefile @@ -47,6 +47,7 @@ LINT_SOURCES = \ test/cpp/morenews.cpp \ test/cpp/converters.cpp \ test/cpp/isolatedata.cpp \ + test/cpp/global.cpp \ test/cpp/makecallback.cpp \ test/cpp/morenews.cpp \ test/cpp/multifile1.cpp \ diff --git a/nan_callbacks_12_inl.h b/nan_callbacks_12_inl.h index bea12c71..6774813a 100644 --- a/nan_callbacks_12_inl.h +++ b/nan_callbacks_12_inl.h @@ -38,7 +38,6 @@ class ReturnValue { value_.Set(handle); #else value_.Set(*reinterpret_cast*>(&handle)); - const_cast &>(handle).Reset(); #endif } diff --git a/nan_callbacks_pre_12_inl.h b/nan_callbacks_pre_12_inl.h index 5e2b8e2b..352b94ac 100644 --- a/nan_callbacks_pre_12_inl.h +++ b/nan_callbacks_pre_12_inl.h @@ -40,7 +40,6 @@ class ReturnValue { TYPE_CHECK(T, S); value_->Dispose(); *value_ = v8::Persistent::New(handle.persistent); - const_cast &>(handle).Reset(); } // Fast primitive setters diff --git a/nan_maybe_pre_43_inl.h b/nan_maybe_pre_43_inl.h index c5386875..3d72f61b 100644 --- a/nan_maybe_pre_43_inl.h +++ b/nan_maybe_pre_43_inl.h @@ -16,10 +16,11 @@ class MaybeLocal { template # if NODE_MODULE_VERSION >= NODE_0_12_MODULE_VERSION - inline MaybeLocal(v8::Local that) : val_(that) {} + inline MaybeLocal(v8::Local that) : // NOLINT(runtime/explicit) + val_(that) {} # else - inline MaybeLocal(v8::Local that) : - val_(*reinterpret_cast*>(&that)) {} + inline MaybeLocal(v8::Local that) : // NOLINT(runtime/explicit) + val_(*reinterpret_cast*>(&that)) { TYPE_CHECK(T, S); } # endif inline bool IsEmpty() const { return val_.IsEmpty(); } diff --git a/nan_persistent_12_inl.h b/nan_persistent_12_inl.h index eb4ff109..89a38c9c 100644 --- a/nan_persistent_12_inl.h +++ b/nan_persistent_12_inl.h @@ -14,25 +14,31 @@ template class Persistent : public: inline Persistent() : v8::Persistent() {} - template inline Persistent(v8::Local that) : + template inline explicit Persistent(v8::Local that) : v8::Persistent(v8::Isolate::GetCurrent(), that) {} template - inline Persistent(const v8::Persistent &that) : + inline explicit Persistent(const v8::Persistent &that) : v8::Persistent(v8::Isolate::GetCurrent(), that) {} inline void Reset() { v8::PersistentBase::Reset(); } - template + template inline void Reset(const v8::Local &other) { v8::PersistentBase::Reset(v8::Isolate::GetCurrent(), other); } - template + template inline void Reset(const v8::PersistentBase &other) { v8::PersistentBase::Reset(v8::Isolate::GetCurrent(), other); } +#if NODE_MODULE_VERSION == NODE_0_12_MODULE_VERSION + inline void Empty() { + v8::Persistent::ClearAndLeak(); + } +#endif + template inline void SetWeak( P *parameter @@ -40,8 +46,6 @@ template class Persistent : , WeakCallbackType type); private: - inline T *operator*() const { return *PersistentBase::persistent; } - template inline void Copy(const Persistent &that) { TYPE_CHECK(T, S); @@ -62,11 +66,11 @@ class Global : public v8::Global { public: inline Global() : v8::Global() {} - template inline Global(v8::Local that) : + template inline explicit Global(v8::Local that) : v8::Global(v8::Isolate::GetCurrent(), that) {} template - inline Global(const v8::PersistentBase &that) : + inline explicit Global(const v8::PersistentBase &that) : v8::Global(v8::Isolate::GetCurrent(), that) {} inline void Reset() { v8::PersistentBase::Reset(); } @@ -81,47 +85,80 @@ class Global : public v8::Global { v8::PersistentBase::Reset(v8::Isolate::GetCurrent(), other); } + Global Pass() { return static_cast(*this); } // NOLINT(build/c++11) + template inline void SetWeak( P *parameter , typename WeakCallbackInfo

::Callback callback , WeakCallbackType type) { - reinterpret_cast*>(this)->SetWeak( - parameter, callback, type); + static_cast*>(static_cast*>(this))-> + SetWeak(parameter, callback, type); } }; #else template class Global : public v8::UniquePersistent { + struct RValue { + inline explicit RValue(v8::UniquePersistent *obj) : object_(obj) {} + v8::UniquePersistent *object_; + }; + public: inline Global() : v8::UniquePersistent() {} - template inline Global(v8::Local that) : + template inline explicit Global(v8::Local that) : v8::UniquePersistent(v8::Isolate::GetCurrent(), that) {} template - inline Global(const v8::PersistentBase &that) : + inline explicit Global(const v8::PersistentBase &that) : v8::UniquePersistent(v8::Isolate::GetCurrent(), that) {} inline void Reset() { v8::PersistentBase::Reset(); } - template + template inline void Reset(const v8::Local &other) { v8::PersistentBase::Reset(v8::Isolate::GetCurrent(), other); } - template + template inline void Reset(const v8::PersistentBase &other) { v8::PersistentBase::Reset(v8::Isolate::GetCurrent(), other); } +#if NODE_MODULE_VERSION == NODE_0_12_MODULE_VERSION + inline void Empty() { + static_cast*>(static_cast*>(this))-> + ClearAndLeak(); + } +#endif + + inline Global(RValue rvalue) { // NOLINT(runtime/explicit) + std::memcpy(this, rvalue.object_, sizeof (*rvalue.object_)); +# if NODE_MODULE_VERSION > NODE_0_12_MODULE_VERSION + rvalue.object_->Empty(); +# else + static_cast*>(static_cast*>( + rvalue.object_))->ClearAndLeak(); +# endif + } + + template + inline Global &operator=(v8::UniquePersistent other) { + return static_cast(v8::UniquePersistent::operator=(other)); + } + + inline operator RValue() { return RValue(this); } + + Global Pass() { return Global(RValue(this)); } + template inline void SetWeak( P *parameter , typename WeakCallbackInfo

::Callback callback , WeakCallbackType type) { - reinterpret_cast*>(this)->SetWeak( - parameter, callback, type); + static_cast*>(static_cast*>(this)) + ->SetWeak(parameter, callback, type); } }; #endif diff --git a/nan_persistent_pre_12_inl.h b/nan_persistent_pre_12_inl.h index d466e34c..88f3a81c 100644 --- a/nan_persistent_pre_12_inl.h +++ b/nan_persistent_pre_12_inl.h @@ -196,32 +196,35 @@ class Global : public PersistentBase { inline Global() : PersistentBase(0) { } template - inline Global(v8::Local that) + inline explicit Global(v8::Local that) : PersistentBase(v8::Persistent::New(that)) { TYPE_CHECK(T, S); } template - inline Global(const PersistentBase &that) - : PersistentBase(that) { + inline explicit Global(const PersistentBase &that) : + PersistentBase(that) { TYPE_CHECK(T, S); } /** * Move constructor. */ - inline Global(RValue rvalue) + inline Global(RValue rvalue) // NOLINT(runtime/explicit) : PersistentBase(rvalue.object->persistent) { - rvalue.object->Reset(); + rvalue.object->Empty(); } inline ~Global() { this->Reset(); } /** * Move via assignment. */ template - inline Global &operator=(Global rhs) { + inline Global &operator=(Global other) { TYPE_CHECK(T, S); - this->Reset(rhs.persistent); - rhs.Reset(); + if (!this->persistent.IsEmpty()) { + this->persistent.Dispose(); + } + this->persistent = other.persistent; + other.Empty(); return *this; } /** diff --git a/test/binding.gyp b/test/binding.gyp index afc2882c..bd548835 100644 --- a/test/binding.gyp +++ b/test/binding.gyp @@ -148,4 +148,8 @@ "target_name" : "typedarrays" , "sources" : [ "cpp/typedarrays.cpp" ] } + , { + "target_name" : "global" + , "sources" : [ "cpp/global.cpp" ] + } ]} diff --git a/test/cpp/global.cpp b/test/cpp/global.cpp new file mode 100644 index 00000000..112f7c8b --- /dev/null +++ b/test/cpp/global.cpp @@ -0,0 +1,75 @@ +/********************************************************************* + * NAN - Native Abstractions for Node.js + * + * Copyright (c) 2016 NAN contributors + * + * MIT License + ********************************************************************/ + +#include + +using namespace Nan; // NOLINT(build/namespaces) + +template Global passer(v8::Local handle) { + return Global(handle).Pass(); +} + +NAN_METHOD(PassGlobal) { + info.GetReturnValue().Set(passer(New(42))); +} + +NAN_METHOD(EmptyGlobal) { + Global g(New("value").ToLocalChecked()); + bool b1 = !g.IsEmpty(); + g.Empty(); // this will leak, never do it + info.GetReturnValue().Set(b1 && g.IsEmpty()); +} + +NAN_METHOD(MoveConstructGlobal) { + Global g(Global(New("value").ToLocalChecked())); + info.GetReturnValue().Set(!g.IsEmpty()); +} + +NAN_METHOD(CopyConstructGlobal) { + Persistent p(New("value").ToLocalChecked()); + bool b1 = !p.IsEmpty(); + Global g(p); + bool b2 = !p.IsEmpty(); + p.Reset(); + info.GetReturnValue().Set(b1 && b2 && !g.IsEmpty()); +} + +NAN_METHOD(MoveAssignGlobal) { + Global g = passer(New("value").ToLocalChecked()); + info.GetReturnValue().Set(!g.IsEmpty()); +} + +NAN_MODULE_INIT(Init) { + Set(target + , New("passGlobal").ToLocalChecked() + , GetFunction(New(PassGlobal)) + .ToLocalChecked() + ); + Set(target + , New("emptyGlobal").ToLocalChecked() + , GetFunction(New(EmptyGlobal)) + .ToLocalChecked() + ); + Set(target + , New("moveConstructGlobal").ToLocalChecked() + , GetFunction(New(MoveConstructGlobal)) + .ToLocalChecked() + ); + Set(target + , New("copyConstructGlobal").ToLocalChecked() + , GetFunction(New(CopyConstructGlobal)) + .ToLocalChecked() + ); + Set(target + , New("moveAssignGlobal").ToLocalChecked() + , GetFunction(New(MoveAssignGlobal)) + .ToLocalChecked() + ); +} + +NODE_MODULE(global, Init) diff --git a/test/cpp/persistent.cpp b/test/cpp/persistent.cpp index e28b17ce..4d3504a3 100644 --- a/test/cpp/persistent.cpp +++ b/test/cpp/persistent.cpp @@ -12,6 +12,7 @@ using namespace Nan; // NOLINT(build/namespaces) static Persistent persistentTest1; +static Persistent persistentTest2; NAN_METHOD(Save1) { persistentTest1.Reset(info[0].As()); @@ -55,6 +56,39 @@ NAN_METHOD(PassGlobal) { info.GetReturnValue().Set(passer(New(42))); } +NAN_METHOD(EmptyPersistent) { + persistentTest2.Reset(New("value").ToLocalChecked()); + bool b1 = !persistentTest2.IsEmpty(); + persistentTest2.Empty(); // this will leak, never do it + info.GetReturnValue().Set(b1 && persistentTest2.IsEmpty()); +} + +NAN_METHOD(EmptyGlobal) { + Global g(New("value").ToLocalChecked()); + bool b1 = !g.IsEmpty(); + g.Empty(); // this will leak, never do it + info.GetReturnValue().Set(b1 && g.IsEmpty()); +} + +NAN_METHOD(MoveConstructGlobal) { + Global g(Global(New("value").ToLocalChecked())); + info.GetReturnValue().Set(!g.IsEmpty()); +} + +NAN_METHOD(CopyConstructGlobal) { + Persistent p(New("value").ToLocalChecked()); + bool b1 = !p.IsEmpty(); + Global g(p); + bool b2 = !p.IsEmpty(); + p.Reset(); + info.GetReturnValue().Set(b1 && b2 && !g.IsEmpty()); +} + +NAN_METHOD(MoveAssignGlobal) { + Global g = passer(New("value").ToLocalChecked()); + info.GetReturnValue().Set(!g.IsEmpty()); +} + NAN_MODULE_INIT(Init) { Set(target , New("save1").ToLocalChecked() @@ -88,6 +122,31 @@ NAN_MODULE_INIT(Init) { , GetFunction(New(PassGlobal)) .ToLocalChecked() ); + Set(target + , New("emptyPersistent").ToLocalChecked() + , GetFunction(New(EmptyPersistent)) + .ToLocalChecked() + ); + Set(target + , New("emptyGlobal").ToLocalChecked() + , GetFunction(New(EmptyGlobal)) + .ToLocalChecked() + ); + Set(target + , New("moveConstructGlobal").ToLocalChecked() + , GetFunction(New(MoveConstructGlobal)) + .ToLocalChecked() + ); + Set(target + , New("copyConstructGlobal").ToLocalChecked() + , GetFunction(New(CopyConstructGlobal)) + .ToLocalChecked() + ); + Set(target + , New("moveAssignGlobal").ToLocalChecked() + , GetFunction(New(MoveAssignGlobal)) + .ToLocalChecked() + ); } NODE_MODULE(persistent, Init) diff --git a/test/cpp/weak.cpp b/test/cpp/weak.cpp index fcf7960a..cc0ad509 100644 --- a/test/cpp/weak.cpp +++ b/test/cpp/weak.cpp @@ -11,6 +11,7 @@ using namespace Nan; // NOLINT(build/namespaces) static Persistent cb; +static Global gcb; void weakCallback( const WeakCallbackInfo &data) { // NOLINT(runtime/references) @@ -30,16 +31,35 @@ v8::Local wrap(v8::Local func) { return scope.Escape(lstring); } +v8::Local gwrap(v8::Local func) { + EscapableHandleScope scope; + v8::Local lstring = New("result").ToLocalChecked(); + int *parameter = new int(42); + gcb.Reset(func); + gcb.SetWeak(parameter, weakCallback, WeakCallbackType::kParameter); + assert(gcb.IsWeak()); + return scope.Escape(lstring); +} + NAN_METHOD(Hustle) { cb.Reset(info[1].As()); info.GetReturnValue().Set(wrap(info[0].As())); } +NAN_METHOD(HustleGlobal) { + cb.Reset(info[1].As()); + info.GetReturnValue().Set(gwrap(info[0].As())); +} + NAN_MODULE_INIT(Init) { Set(target , New("hustle").ToLocalChecked() , New(Hustle)->GetFunction() ); + Set(target + , New("hustleGlobal").ToLocalChecked() + , New(HustleGlobal)->GetFunction() + ); } NODE_MODULE(weak, Init) diff --git a/test/js/global-test.js b/test/js/global-test.js new file mode 100644 index 00000000..eea19c05 --- /dev/null +++ b/test/js/global-test.js @@ -0,0 +1,28 @@ +/********************************************************************* + * NAN - Native Abstractions for Node.js + * + * Copyright (c) 2016 NAN contributors + * + * MIT License + ********************************************************************/ + +const test = require('tap').test + , testRoot = require('path').resolve(__dirname, '..') + , bindings = require('bindings')({ module_root: testRoot, bindings: 'global' }); + +test('global', function (t) { + t.plan(10); + + var global_ = bindings; + t.type(global_.passGlobal, 'function'); + t.type(global_.emptyGlobal, 'function'); + t.type(global_.moveConstructGlobal, 'function'); + t.type(global_.copyConstructGlobal, 'function'); + t.type(global_.moveAssignGlobal, 'function'); + + t.equal(global_.passGlobal(), 42, 'pass global'); + t.ok(global_.emptyGlobal()); + t.ok(global_.moveConstructGlobal()); + t.ok(global_.copyConstructGlobal()); + t.ok(global_.moveAssignGlobal()); +}); diff --git a/test/js/persistent-test.js b/test/js/persistent-test.js index b7dd33ff..3707d912 100644 --- a/test/js/persistent-test.js +++ b/test/js/persistent-test.js @@ -20,7 +20,9 @@ test('persistent', function (t) { t.type(persistent.toPersistentAndBackAgain, 'function'); t.type(persistent.persistentToPersistent, 'function'); t.type(persistent.copyablePersistent, 'function'); - t.type(persistent.passGlobal, 'function'); + t.type(persistent.emptyPersistent, 'function'); + + t.ok(persistent.emptyPersistent()); t.deepEqual(persistent.toPersistentAndBackAgain({ x: 42 }), { x: 42 }); @@ -30,8 +32,6 @@ test('persistent', function (t) { t.equal(persistent.get1(), 'a string to save'); t.equal(persistent.copyablePersistent(), 'a string to save'); - t.equal(persistent.passGlobal(), 42, 'pass global'); - setTimeout(function () { t.equal(persistent.get1(), 'a string to save'); persistent.dispose1(); diff --git a/test/js/weak-test.js b/test/js/weak-test.js index 5c91590a..4c842aaa 100644 --- a/test/js/weak-test.js +++ b/test/js/weak-test.js @@ -11,10 +11,11 @@ const test = require('tap').test , bindings = require('bindings')({ module_root: testRoot, bindings: 'weak' }); test('weak', function (t) { - t.plan(3); + t.plan(6); var weak = bindings, count = 0; t.type(weak.hustle, 'function'); + t.type(weak.hustleGlobal, 'function'); weak.hustle(function () {}, function (val) { t.equal(val, 42); @@ -27,5 +28,20 @@ test('weak', function (t) { // do not run weak callback gc(); + t.equal(count, 1); + + count = 0; + + weak.hustleGlobal(function () {}, function (val) { + t.equal(val, 42); + count++; + }); + + // run weak callback, should dispose + gc(); + + // do not run weak callback + gc(); + t.equal(count, 1); });