From d120d92bfef0b5012e76c636335fee80e9c1e4a9 Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Mon, 4 Nov 2013 10:49:55 -0800 Subject: [PATCH] base-object: add BaseObject BaseObject is a class that just handles the Persistent handle attached to the class instance. This also removed WeakObject. Reordering the inheritance chain helps prevent unneeded calls on instances that don't call MakeCallback. --- node.gyp | 4 +- src/async-wrap-inl.h | 24 +-- src/async-wrap.h | 13 +- src/{weak-object-inl.h => base-object-inl.h} | 71 +++++---- src/{weak-object.h => base-object.h} | 42 ++++-- src/node_contextify.cc | 9 +- src/node_crypto.cc | 2 + src/node_crypto.h | 147 ++++++++++--------- src/node_http_parser.cc | 9 +- src/node_stat_watcher.cc | 9 +- src/node_stat_watcher.h | 7 +- src/node_zlib.cc | 11 +- 12 files changed, 188 insertions(+), 160 deletions(-) rename src/{weak-object-inl.h => base-object-inl.h} (50%) rename src/{weak-object.h => base-object.h} (64%) diff --git a/node.gyp b/node.gyp index 4bb73980d7ce13..095ba84fdb745a 100644 --- a/node.gyp +++ b/node.gyp @@ -117,6 +117,8 @@ # headers to make for a more pleasant IDE experience 'src/async-wrap.h', 'src/async-wrap-inl.h', + 'src/base-object.h', + 'src/base-object-inl.h', 'src/env.h', 'src/env-inl.h', 'src/handle_wrap.h', @@ -145,8 +147,6 @@ 'src/tree.h', 'src/util.h', 'src/util-inl.h', - 'src/weak-object.h', - 'src/weak-object-inl.h', 'deps/http_parser/http_parser.h', '<(SHARED_INTERMEDIATE_DIR)/node_natives.h', # javascript files to make for an even more pleasant IDE experience diff --git a/src/async-wrap-inl.h b/src/async-wrap-inl.h index 78e297a032cd5b..8408e021450e10 100644 --- a/src/async-wrap-inl.h +++ b/src/async-wrap-inl.h @@ -23,21 +23,21 @@ #define SRC_ASYNC_WRAP_INL_H_ #include "async-wrap.h" +#include "base-object.h" +#include "base-object-inl.h" #include "env.h" #include "env-inl.h" #include "util.h" #include "util-inl.h" + #include "v8.h" #include namespace node { inline AsyncWrap::AsyncWrap(Environment* env, v8::Handle object) - : object_(env->isolate(), object), - env_(env), + : BaseObject(env, object), async_flags_(NO_OPTIONS) { - assert(!object.IsEmpty()); - if (!env->has_async_listeners()) return; @@ -54,7 +54,6 @@ inline AsyncWrap::AsyncWrap(Environment* env, v8::Handle object) inline AsyncWrap::~AsyncWrap() { - assert(persistent().IsEmpty()); } @@ -89,21 +88,6 @@ inline bool AsyncWrap::has_async_queue() { } -inline Environment* AsyncWrap::env() const { - return env_; -} - - -inline v8::Local AsyncWrap::object() { - return PersistentToLocal(env()->isolate(), persistent()); -} - - -inline v8::Persistent& AsyncWrap::persistent() { - return object_; -} - - inline v8::Handle AsyncWrap::MakeCallback( const v8::Handle cb, int argc, diff --git a/src/async-wrap.h b/src/async-wrap.h index 5bf38aae03d12f..b978ae3c5bc0c3 100644 --- a/src/async-wrap.h +++ b/src/async-wrap.h @@ -22,12 +22,13 @@ #ifndef SRC_ASYNC_WRAP_H_ #define SRC_ASYNC_WRAP_H_ +#include "base-object.h" #include "env.h" #include "v8.h" namespace node { -class AsyncWrap { +class AsyncWrap : public BaseObject { public: enum AsyncFlags { NO_OPTIONS = 0, @@ -49,14 +50,6 @@ class AsyncWrap { inline bool has_async_queue(); - inline Environment* env() const; - - // Returns the wrapped object. Illegal to call in your destructor. - inline v8::Local object(); - - // Parent class is responsible to Dispose. - inline v8::Persistent& persistent(); - // Only call these within a valid HandleScope. inline v8::Handle MakeCallback(const v8::Handle cb, int argc, @@ -79,8 +72,6 @@ class AsyncWrap { static inline void RemoveAsyncListener( const v8::FunctionCallbackInfo& args); - v8::Persistent object_; - Environment* const env_; uint32_t async_flags_; }; diff --git a/src/weak-object-inl.h b/src/base-object-inl.h similarity index 50% rename from src/weak-object-inl.h rename to src/base-object-inl.h index c35cb9dbeeb30f..4d726df7ca7989 100644 --- a/src/weak-object-inl.h +++ b/src/base-object-inl.h @@ -19,48 +19,69 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. -#ifndef SRC_WEAK_OBJECT_INL_H_ -#define SRC_WEAK_OBJECT_INL_H_ +#ifndef SRC_BASE_OBJECT_INL_H_ +#define SRC_BASE_OBJECT_INL_H_ -#include "weak-object.h" -#include "async-wrap.h" -#include "async-wrap-inl.h" +#include "base-object.h" #include "util.h" #include "util-inl.h" +#include "v8.h" + +#include namespace node { -WeakObject::WeakObject(Environment* env, v8::Local object) - : AsyncWrap(env, object) { - persistent().MarkIndependent(); +inline BaseObject::BaseObject(Environment* env, v8::Local handle) + : handle_(env->isolate(), handle), + env_(env) { + assert(!handle.IsEmpty()); +} + - // The pointer is resolved as void*. - Wrap(object, this); - MakeWeak(); +inline BaseObject::~BaseObject() { + assert(handle_.IsEmpty()); } -WeakObject::~WeakObject() { + +inline v8::Persistent& BaseObject::persistent() { + return handle_; } -void WeakObject::MakeWeak() { - persistent().MakeWeak(this, WeakCallback); + +inline v8::Local BaseObject::object() { + return PersistentToLocal(env_->isolate(), handle_); } -void WeakObject::ClearWeak() { - persistent().ClearWeak(); + +inline Environment* BaseObject::env() const { + return env_; } -void WeakObject::WeakCallback(v8::Isolate* isolate, - v8::Persistent* persistent, - WeakObject* self) { - // Dispose now instead of in the destructor to avoid child classes that call - // `delete this` in their destructor from blowing up. - // Dispose the class member instead of the argument or else the IsEmpty() - // check in ~AsyncWrap will fail. - self->persistent().Dispose(); + +template +inline void BaseObject::WeakCallback( + const v8::WeakCallbackData& data) { + Type* self = data.GetParameter(); + self->persistent().Reset(); delete self; } + +template +inline void BaseObject::MakeWeak(Type* ptr) { + v8::HandleScope scope(env_->isolate()); + v8::Local handle = object(); + assert(handle->InternalFieldCount() > 0); + Wrap(handle, ptr); + handle_.MarkIndependent(); + handle_.SetWeak(ptr, WeakCallback); +} + + +inline void BaseObject::ClearWeak() { + handle_.ClearWeak(); +} + } // namespace node -#endif // SRC_WEAK_OBJECT_INL_H_ +#endif // SRC_BASE_OBJECT_INL_H_ diff --git a/src/weak-object.h b/src/base-object.h similarity index 64% rename from src/weak-object.h rename to src/base-object.h index 829d4128afaa7b..8b3f6da594d601 100644 --- a/src/weak-object.h +++ b/src/base-object.h @@ -19,29 +19,43 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. -#ifndef SRC_WEAK_OBJECT_H_ -#define SRC_WEAK_OBJECT_H_ +#ifndef SRC_BASE_OBJECT_H_ +#define SRC_BASE_OBJECT_H_ -#include "async-wrap.h" #include "env.h" #include "v8.h" namespace node { -class WeakObject : public AsyncWrap { - protected: - // |object| should be an instance of a v8::ObjectTemplate that has at least - // one internal field reserved with v8::ObjectTemplate::SetInternalFieldCount. - inline WeakObject(Environment* env, v8::Local object); - virtual inline ~WeakObject(); - inline void MakeWeak(); +class BaseObject { + public: + BaseObject(Environment* env, v8::Local handle); + ~BaseObject(); + + // Returns the wrapped object. Illegal to call in your destructor. + inline v8::Local object(); + + // Parent class is responsible to Dispose. + inline v8::Persistent& persistent(); + + inline Environment* env() const; + + template + inline void MakeWeak(Type* ptr); + inline void ClearWeak(); + private: - inline static void WeakCallback(v8::Isolate* isolate, - v8::Persistent* persistent, - WeakObject* self); + BaseObject(); + + template + static inline void WeakCallback( + const v8::WeakCallbackData& data); + + v8::Persistent handle_; + Environment* env_; }; } // namespace node -#endif // SRC_WEAK_OBJECT_H_ +#endif // SRC_BASE_OBJECT_H_ diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 508e32957256c9..b3316f54d21fbb 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -22,12 +22,12 @@ #include "node.h" #include "node_internals.h" #include "node_watchdog.h" +#include "base-object.h" +#include "base-object-inl.h" #include "env.h" #include "env-inl.h" #include "util.h" #include "util-inl.h" -#include "weak-object.h" -#include "weak-object-inl.h" namespace node { @@ -381,7 +381,7 @@ class ContextifyContext { } }; -class ContextifyScript : public WeakObject { +class ContextifyScript : public BaseObject { private: Persistent