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

@@ -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

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 <assert.h>
 
 namespace node {
 
 inline AsyncWrap::AsyncWrap(Environment* env, v8::Handle<v8::Object> 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<v8::Object> 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<v8::Object> AsyncWrap::object() {
-  return PersistentToLocal(env()->isolate(), persistent());
-}
-
-
-inline v8::Persistent<v8::Object>& AsyncWrap::persistent() {
-  return object_;
-}
-
-
 inline v8::Handle<v8::Value> AsyncWrap::MakeCallback(
     const v8::Handle<v8::Function> cb,
     int argc,

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<v8::Object> object();
-
-  // Parent class is responsible to Dispose.
-  inline v8::Persistent<v8::Object>& persistent();
-
   // Only call these within a valid HandleScope.
   inline v8::Handle<v8::Value> MakeCallback(const v8::Handle<v8::Function> cb,
                                             int argc,
@@ -79,8 +72,6 @@ class AsyncWrap {
   static inline void RemoveAsyncListener(
       const v8::FunctionCallbackInfo<v8::Value>& args);
 
-  v8::Persistent<v8::Object> object_;
-  Environment* const env_;
   uint32_t async_flags_;
 };
 

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 <assert.h>
 
 namespace node {
 
-WeakObject::WeakObject(Environment* env, v8::Local<v8::Object> object)
-    : AsyncWrap(env, object) {
-  persistent().MarkIndependent();
-
-  // The pointer is resolved as void*.
-  Wrap<WeakObject>(object, this);
-  MakeWeak();
+inline BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> handle)
+    : handle_(env->isolate(), handle),
+      env_(env) {
+  assert(!handle.IsEmpty());
 }
 
-WeakObject::~WeakObject() {
+
+inline BaseObject::~BaseObject() {
+  assert(handle_.IsEmpty());
 }
 
-void WeakObject::MakeWeak() {
-  persistent().MakeWeak(this, WeakCallback);
+
+inline v8::Persistent<v8::Object>& BaseObject::persistent() {
+  return handle_;
 }
 
-void WeakObject::ClearWeak() {
-  persistent().ClearWeak();
+
+inline v8::Local<v8::Object> BaseObject::object() {
+  return PersistentToLocal(env_->isolate(), handle_);
 }
 
-void WeakObject::WeakCallback(v8::Isolate* isolate,
-                              v8::Persistent<v8::Object>* 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();
+
+inline Environment* BaseObject::env() const {
+  return env_;
+}
+
+
+template <typename Type>
+inline void BaseObject::WeakCallback(
+    const v8::WeakCallbackData<v8::Object, Type>& data) {
+  Type* self = data.GetParameter();
+  self->persistent().Reset();
   delete self;
 }
 
+
+template <typename Type>
+inline void BaseObject::MakeWeak(Type* ptr) {
+  v8::HandleScope scope(env_->isolate());
+  v8::Local<v8::Object> handle = object();
+  assert(handle->InternalFieldCount() > 0);
+  Wrap<Type>(handle, ptr);
+  handle_.MarkIndependent();
+  handle_.SetWeak<Type>(ptr, WeakCallback<Type>);
+}
+
+
+inline void BaseObject::ClearWeak() {
+  handle_.ClearWeak();
+}
+
 }  // namespace node
 
-#endif  // SRC_WEAK_OBJECT_INL_H_
+#endif  // SRC_BASE_OBJECT_INL_H_

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<v8::Object> object);
-  virtual inline ~WeakObject();
-  inline void MakeWeak();
+class BaseObject {
+ public:
+  BaseObject(Environment* env, v8::Local<v8::Object> handle);
+  ~BaseObject();
+
+  // Returns the wrapped object.  Illegal to call in your destructor.
+  inline v8::Local<v8::Object> object();
+
+  // Parent class is responsible to Dispose.
+  inline v8::Persistent<v8::Object>& persistent();
+
+  inline Environment* env() const;
+
+  template <typename Type>
+  inline void MakeWeak(Type* ptr);
+
   inline void ClearWeak();
+
  private:
-  inline static void WeakCallback(v8::Isolate* isolate,
-                                  v8::Persistent<v8::Object>* persistent,
-                                  WeakObject* self);
+  BaseObject();
+
+  template <typename Type>
+  static inline void WeakCallback(
+      const v8::WeakCallbackData<v8::Object, Type>& data);
+
+  v8::Persistent<v8::Object> handle_;
+  Environment* env_;
 };
 
 }  // namespace node
 
-#endif  // SRC_WEAK_OBJECT_H_
+#endif  // SRC_BASE_OBJECT_H_

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<Script> script_;
 
@@ -607,7 +607,8 @@ class ContextifyScript : public WeakObject {
 
 
   ContextifyScript(Environment* env, Local<Object> object)
-      : WeakObject(env, object) {
+      : BaseObject(env, object) {
+    MakeWeak<ContextifyScript>(this);
   }
 
 

src/node_crypto.cc

@@ -26,6 +26,8 @@
 #include "node_crypto_groups.h"
 #include "tls_wrap.h"  // TLSCallbacks
 
+#include "async-wrap.h"
+#include "async-wrap-inl.h"
 #include "env.h"
 #include "env-inl.h"
 #include "string_bytes.h"

src/node_crypto.h

@@ -31,8 +31,10 @@
 #endif
 
 #include "env.h"
-#include "weak-object.h"
-#include "weak-object-inl.h"
+#include "async-wrap.h"
+#include "async-wrap-inl.h"
+#include "base-object.h"
+#include "base-object-inl.h"
 
 #include "v8.h"
 
@@ -59,8 +61,12 @@ extern X509_STORE* root_cert_store;
 // Forward declaration
 class Connection;
 
-class SecureContext : public WeakObject {
+class SecureContext : public BaseObject {
  public:
+  ~SecureContext() {
+    FreeCTXMem();
+  }
+
   static void Initialize(Environment* env, v8::Handle<v8::Object> target);
 
   X509_STORE* ca_store_;
@@ -90,9 +96,10 @@ class SecureContext : public WeakObject {
   static void SetTicketKeys(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   SecureContext(Environment* env, v8::Local<v8::Object> wrap)
-      : WeakObject(env, wrap),
+      : BaseObject(env, wrap),
         ca_store_(NULL),
         ctx_(NULL) {
+    MakeWeak<SecureContext>(this);
   }
 
   void FreeCTXMem() {
@@ -111,10 +118,6 @@ class SecureContext : public WeakObject {
       assert(ca_store_ == NULL);
     }
   }
-
-  ~SecureContext() {
-    FreeCTXMem();
-  }
 };
 
 template <class Base>
@@ -214,8 +217,16 @@ class SSLWrap {
   friend class SecureContext;
 };
 
-class Connection : public SSLWrap<Connection>, public WeakObject {
+class Connection : public SSLWrap<Connection>, public AsyncWrap {
  public:
+  ~Connection() {
+#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
+    sniObject_.Dispose();
+    sniContext_.Dispose();
+    servername_.Dispose();
+#endif
+  }
+
   static void Initialize(Environment* env, v8::Handle<v8::Object> target);
 
 #ifdef OPENSSL_NPN_NEGOTIATED
@@ -272,24 +283,17 @@ class Connection : public SSLWrap<Connection>, public WeakObject {
              SecureContext* sc,
              SSLWrap<Connection>::Kind kind)
       : SSLWrap<Connection>(env, sc, kind),
-        WeakObject(env, wrap),
+        AsyncWrap(env, wrap),
         bio_read_(NULL),
         bio_write_(NULL),
         hello_offset_(0) {
+    MakeWeak<Connection>(this);
     hello_parser_.Start(SSLWrap<Connection>::OnClientHello,
                         OnClientHelloParseEnd,
                         this);
     enable_session_callbacks();
   }
 
-  ~Connection() {
-#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
-    sniObject_.Dispose();
-    sniContext_.Dispose();
-    servername_.Dispose();
-#endif
-  }
-
  private:
   static void SSLInfoCallback(const SSL *ssl, int where, int ret);
 
@@ -303,8 +307,14 @@ class Connection : public SSLWrap<Connection>, public WeakObject {
   friend class SecureContext;
 };
 
-class CipherBase : public WeakObject {
+class CipherBase : public BaseObject {
  public:
+  ~CipherBase() {
+    if (!initialised_)
+      return;
+    EVP_CIPHER_CTX_cleanup(&ctx_);
+  }
+
   static void Initialize(Environment* env, v8::Handle<v8::Object> target);
 
  protected:
@@ -333,16 +343,11 @@ class CipherBase : public WeakObject {
   CipherBase(Environment* env,
              v8::Local<v8::Object> wrap,
              CipherKind kind)
-      : WeakObject(env, wrap),
+      : BaseObject(env, wrap),
         cipher_(NULL),
         initialised_(false),
         kind_(kind) {
-  }
-
-  ~CipherBase() {
-    if (!initialised_)
-      return;
-    EVP_CIPHER_CTX_cleanup(&ctx_);
+    MakeWeak<CipherBase>(this);
   }
 
  private:
@@ -352,8 +357,14 @@ class CipherBase : public WeakObject {
   CipherKind kind_;
 };
 
-class Hmac : public WeakObject {
+class Hmac : public BaseObject {
  public:
+  ~Hmac() {
+    if (!initialised_)
+      return;
+    HMAC_CTX_cleanup(&ctx_);
+  }
+
   static void Initialize(Environment* env, v8::Handle<v8::Object> target);
 
  protected:
@@ -367,15 +378,10 @@ class Hmac : public WeakObject {
   static void HmacDigest(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   Hmac(Environment* env, v8::Local<v8::Object> wrap)
-      : WeakObject(env, wrap),
+      : BaseObject(env, wrap),
         md_(NULL),
         initialised_(false) {
-  }
-
-  ~Hmac() {
-    if (!initialised_)
-      return;
-    HMAC_CTX_cleanup(&ctx_);
+    MakeWeak<Hmac>(this);
   }
 
  private:
@@ -384,8 +390,14 @@ class Hmac : public WeakObject {
   bool initialised_;
 };
 
-class Hash : public WeakObject {
+class Hash : public BaseObject {
  public:
+  ~Hash() {
+    if (!initialised_)
+      return;
+    EVP_MD_CTX_cleanup(&mdctx_);
+  }
+
   static void Initialize(Environment* env, v8::Handle<v8::Object> target);
 
   bool HashInit(const char* hash_type);
@@ -397,15 +409,10 @@ class Hash : public WeakObject {
   static void HashDigest(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   Hash(Environment* env, v8::Local<v8::Object> wrap)
-      : WeakObject(env, wrap),
+      : BaseObject(env, wrap),
         md_(NULL),
         initialised_(false) {
-  }
-
-  ~Hash() {
-    if (!initialised_)
-      return;
-    EVP_MD_CTX_cleanup(&mdctx_);
+    MakeWeak<Hash>(this);
   }
 
  private:
@@ -414,8 +421,14 @@ class Hash : public WeakObject {
   bool initialised_;
 };
 
-class Sign : public WeakObject {
+class Sign : public BaseObject {
  public:
+  ~Sign() {
+    if (!initialised_)
+      return;
+    EVP_MD_CTX_cleanup(&mdctx_);
+  }
+
   static void Initialize(Environment* env, v8::Handle<v8::Object> target);
 
   void SignInit(const char* sign_type);
@@ -433,15 +446,10 @@ class Sign : public WeakObject {
   static void SignFinal(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   Sign(Environment* env, v8::Local<v8::Object> wrap)
-      : WeakObject(env, wrap),
+      : BaseObject(env, wrap),
         md_(NULL),
         initialised_(false) {
-  }
-
-  ~Sign() {
-    if (!initialised_)
-      return;
-    EVP_MD_CTX_cleanup(&mdctx_);
+    MakeWeak<Sign>(this);
   }
 
  private:
@@ -450,8 +458,14 @@ class Sign : public WeakObject {
   bool initialised_;
 };
 
-class Verify : public WeakObject {
+class Verify : public BaseObject {
  public:
+  ~Verify() {
+    if (!initialised_)
+      return;
+    EVP_MD_CTX_cleanup(&mdctx_);
+  }
+
   static void Initialize(Environment* env, v8::Handle<v8::Object> target);
 
   void VerifyInit(const char* verify_type);
@@ -468,15 +482,10 @@ class Verify : public WeakObject {
   static void VerifyFinal(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   Verify(Environment* env, v8::Local<v8::Object> wrap)
-      : WeakObject(env, wrap),
+      : BaseObject(env, wrap),
         md_(NULL),
         initialised_(false) {
-  }
-
-  ~Verify() {
-    if (!initialised_)
-      return;
-    EVP_MD_CTX_cleanup(&mdctx_);
+    MakeWeak<Verify>(this);
   }
 
  private:
@@ -485,8 +494,14 @@ class Verify : public WeakObject {
   bool initialised_;
 };
 
-class DiffieHellman : public WeakObject {
+class DiffieHellman : public BaseObject {
  public:
+  ~DiffieHellman() {
+    if (dh != NULL) {
+      DH_free(dh);
+    }
+  }
+
   static void Initialize(Environment* env, v8::Handle<v8::Object> target);
 
   bool Init(int primeLength);
@@ -507,15 +522,10 @@ class DiffieHellman : public WeakObject {
   static void SetPrivateKey(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   DiffieHellman(Environment* env, v8::Local<v8::Object> wrap)
-      : WeakObject(env, wrap),
+      : BaseObject(env, wrap),
         initialised_(false),
         dh(NULL) {
-  }
-
-  ~DiffieHellman() {
-    if (dh != NULL) {
-      DH_free(dh);
-    }
+    MakeWeak<DiffieHellman>(this);
   }
 
  private:
@@ -525,7 +535,7 @@ class DiffieHellman : public WeakObject {
   DH* dh;
 };
 
-class Certificate : public WeakObject {
+class Certificate : public AsyncWrap {
  public:
   static void Initialize(v8::Handle<v8::Object> target);
 
@@ -541,7 +551,8 @@ class Certificate : public WeakObject {
   static void ExportChallenge(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   Certificate(Environment* env, v8::Local<v8::Object> wrap)
-    : WeakObject(env, wrap) {
+      : AsyncWrap(env, wrap) {
+    MakeWeak<Certificate>(this);
   }
 };
 

src/node_http_parser.cc

@@ -23,12 +23,12 @@
 #include "node_buffer.h"
 #include "node_http_parser.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"
 #include "v8.h"
 
 #include <stdlib.h>  // free()
@@ -163,12 +163,13 @@ struct StringPtr {
 };
 
 
-class Parser : public WeakObject {
+class Parser : public BaseObject {
  public:
   Parser(Environment* env, Local<Object> wrap, enum http_parser_type type)
-      : WeakObject(env, wrap),
+      : BaseObject(env, wrap),
         current_buffer_len_(0),
         current_buffer_data_(NULL) {
+    MakeWeak<Parser>(this);
     Init(type);
   }
 

src/node_stat_watcher.cc

@@ -20,12 +20,12 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 #include "node_stat_watcher.h"
+#include "async-wrap.h"
+#include "async-wrap-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"
 
 #include <assert.h>
 #include <string.h>
@@ -66,8 +66,9 @@ static void Delete(uv_handle_t* handle) {
 
 
 StatWatcher::StatWatcher(Environment* env, Local<Object> wrap)
-    : WeakObject(env, wrap),
+    : AsyncWrap(env, wrap),
       watcher_(new uv_fs_poll_t) {
+  MakeWeak<StatWatcher>(this);
   uv_fs_poll_init(env->event_loop(), watcher_);
   watcher_->data = static_cast<void*>(this);
 }
@@ -135,7 +136,7 @@ void StatWatcher::Stop() {
   if (!uv_is_active(reinterpret_cast<uv_handle_t*>(watcher_)))
     return;
   uv_fs_poll_stop(watcher_);
-  MakeWeak();
+  MakeWeak<StatWatcher>(this);
 }
 
 

src/node_stat_watcher.h

@@ -23,20 +23,21 @@
 #define SRC_NODE_STAT_WATCHER_H_
 
 #include "node.h"
+#include "async-wrap.h"
 #include "env.h"
-#include "weak-object.h"
 #include "uv.h"
 #include "v8.h"
 
 namespace node {
 
-class StatWatcher : public WeakObject {
+class StatWatcher : public AsyncWrap {
  public:
+  virtual ~StatWatcher();
+
   static void Initialize(v8::Handle<v8::Object> target);
 
  protected:
   StatWatcher(Environment* env, v8::Local<v8::Object> wrap);
-  virtual ~StatWatcher();
 
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Start(const v8::FunctionCallbackInfo<v8::Value>& args);

src/node_zlib.cc

@@ -22,12 +22,12 @@
 #include "node.h"
 #include "node_buffer.h"
 
+#include "async-wrap.h"
+#include "async-wrap-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"
 
 #include "v8.h"
 #include "zlib.h"
@@ -69,11 +69,11 @@ void InitZlib(v8::Handle<v8::Object> target);
 /**
  * Deflate/Inflate
  */
-class ZCtx : public WeakObject {
+class ZCtx : public AsyncWrap {
  public:
 
   ZCtx(Environment* env, Local<Object> wrap, node_zlib_mode mode)
-      : WeakObject(env, wrap),
+      : AsyncWrap(env, wrap),
         chunk_size_(0),
         dictionary_(NULL),
         dictionary_len_(0),
@@ -87,6 +87,7 @@ class ZCtx : public WeakObject {
         windowBits_(0),
         write_in_progress_(false),
         refs_(0) {
+    MakeWeak<ZCtx>(this);
   }
 
 
@@ -522,7 +523,7 @@ class ZCtx : public WeakObject {
   void Unref() {
     assert(refs_ > 0);
     if (--refs_ == 0) {
-      MakeWeak();
+      MakeWeak<ZCtx>(this);
     }
   }