diff options
author | Anna Henningsen <anna@addaleax.net> | 2018-05-01 18:34:52 +0200 |
---|---|---|
committer | Anna Henningsen <anna@addaleax.net> | 2018-05-04 00:57:39 +0200 |
commit | bd201102862a194f3f5ce669e0a3c8143eafc900 (patch) | |
tree | 22c86fcd4bff3716704c981f14fa80c3a6dd0d6b /src/node_crypto.h | |
parent | a9b0d8235d5ae11a5ae0748f05c4d290a848f1b9 (diff) | |
download | node-new-bd201102862a194f3f5ce669e0a3c8143eafc900.tar.gz |
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>
Diffstat (limited to 'src/node_crypto.h')
-rw-r--r-- | src/node_crypto.h | 16 |
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); } |