src: refactor `BaseObject` internal field management

- Instead of storing a pointer whose type refers to the specific
  subclass of `BaseObject`, just store a `BaseObject*` directly.
  This means in particular that one can cast to classes along
  the way of the inheritance chain without issues, and that
  `BaseObject*` no longer needs to be the first superclass
  in the case of multiple inheritance.

  In particular, this renders hack-y solutions to this problem (like
  ddc19be6de1ba263d9c175b2760696e7b9918b25) obsolete and addresses
  a `TODO` comment of mine.

- Move wrapping/unwrapping methods to the `BaseObject` class.
  We use these almost exclusively for `BaseObject`s, and I hope
  that this gives a better idea of how (and for what) these are used
  in our code.

- Perform initialization/deinitialization of the internal field
  in the `BaseObject*` constructor/destructor. This makes the code
  a bit more obviously correct, avoids explicit calls for this
  in subclass constructors, and in particular allows us to avoid
  crash situations when we previously called `ClearWrap()`
  during GC.

  This also means that we enforce that the object passed to the
  `BaseObject` constructor needs to have an internal field.
  This is the only reason for the test change.

- Change the signature of `MakeWeak()` to not require a pointer
  argument. Previously, this would always have been the same
  as `this`, and no other value made sense. Also, the parameter
  was something that I personally found somewhat confusing
  when becoming familiar with Node’s code.

- Add a `TODO` comment that motivates switching to real inheritance
  for the JS types we expose from the native side. This patch
  brings us a lot closer to being able to do that.

- Some less significant drive-by cleanup.

Since we *effectively* already store the `BaseObject*` pointer
anyway since ddc19be6de1ba263d9c175b2760696e7b9918b25, I do not
think that this is going to have any impact on diagnostic tooling.

Fixes: https://github.com/nodejs/node/issues/18897
PR-URL: https://github.com/nodejs/node/pull/20455
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

1 files changed, 8 insertions, 8 deletions

diff --git a/src/node_crypto.h b/src/node_crypto.h
index 3963f7050f..6f34eae359 100644
--- a/src/node_crypto.h
+++ b/src/node_crypto.h
@@ -175,7 +175,7 @@ class SecureContext : public BaseObject {
         ctx_(nullptr),
         cert_(nullptr),
         issuer_(nullptr) {
-    MakeWeak<SecureContext>(this);
+    MakeWeak();
     env->isolate()->AdjustAmountOfExternalAllocatedMemory(kExternalSize);
   }
 
@@ -403,7 +403,7 @@ class CipherBase : public BaseObject {
         auth_tag_set_(false),
         auth_tag_len_(kNoAuthTagLength),
         pending_auth_failed_(false) {
-    MakeWeak<CipherBase>(this);
+    MakeWeak();
   }
 
  private:
@@ -434,7 +434,7 @@ class Hmac : public BaseObject {
   Hmac(Environment* env, v8::Local<v8::Object> wrap)
       : BaseObject(env, wrap),
         ctx_(nullptr) {
-    MakeWeak<Hmac>(this);
+    MakeWeak();
   }
 
  private:
@@ -459,7 +459,7 @@ class Hash : public BaseObject {
       : BaseObject(env, wrap),
         mdctx_(nullptr),
         finalized_(false) {
-    MakeWeak<Hash>(this);
+    MakeWeak();
   }
 
  private:
@@ -514,7 +514,7 @@ class Sign : public SignBase {
   static void SignFinal(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   Sign(Environment* env, v8::Local<v8::Object> wrap) : SignBase(env, wrap) {
-    MakeWeak<Sign>(this);
+    MakeWeak();
   }
 };
 
@@ -537,7 +537,7 @@ class Verify : public SignBase {
   static void VerifyFinal(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   Verify(Environment* env, v8::Local<v8::Object> wrap) : SignBase(env, wrap) {
-    MakeWeak<Verify>(this);
+    MakeWeak();
   }
 };
 
@@ -605,7 +605,7 @@ class DiffieHellman : public BaseObject {
         initialised_(false),
         verifyError_(0),
         dh(nullptr) {
-    MakeWeak<DiffieHellman>(this);
+    MakeWeak();
   }
 
  private:
@@ -641,7 +641,7 @@ class ECDH : public BaseObject {
       : BaseObject(env, wrap),
         key_(key),
         group_(EC_KEY_get0_group(key_)) {
-    MakeWeak<ECDH>(this);
+    MakeWeak();
     CHECK_NE(group_, nullptr);
   }