From 9e2ceab5ce89fa478c7087133ce1eb936bdd7bc8 Mon Sep 17 00:00:00 2001 From: ConorDavenport Date: Tue, 28 Jan 2020 16:17:26 +0000 Subject: [PATCH] src: remove duplicate field env in CryptoJob class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removed field env from cryptojob class, replaced with function env() inherited from ThreadPoolWork PR-URL: https://github.com/nodejs/node/pull/31554 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Benjamin Gruenbaum Reviewed-By: Colin Ihrig Reviewed-By: David Carlier Reviewed-By: Rich Trott Reviewed-By: Tobias Nießen --- src/node_crypto.cc | 42 +++++++++++++++++++++--------------------- src/node_internals.h | 2 ++ src/node_zlib.cc | 26 ++++++++++++++------------ 3 files changed, 37 insertions(+), 33 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 3bb047347125f3..8c9d93ca435598 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -6192,9 +6192,8 @@ bool ECDH::IsKeyPairValid() { // TODO(addaleax): If there is an `AsyncWrap`, it currently has no access to // this object. This makes proper reporting of memory usage impossible. struct CryptoJob : public ThreadPoolWork { - Environment* const env; std::unique_ptr async_wrap; - inline explicit CryptoJob(Environment* env) : ThreadPoolWork(env), env(env) {} + inline explicit CryptoJob(Environment* env) : ThreadPoolWork(env) {} inline void AfterThreadPoolWork(int status) final; virtual void AfterThreadPoolWork() = 0; static inline void Run(std::unique_ptr job, Local wrap); @@ -6205,8 +6204,8 @@ void CryptoJob::AfterThreadPoolWork(int status) { CHECK(status == 0 || status == UV_ECANCELED); std::unique_ptr job(this); if (status == UV_ECANCELED) return; - HandleScope handle_scope(env->isolate()); - Context::Scope context_scope(env->context()); + HandleScope handle_scope(env()->isolate()); + Context::Scope context_scope(env()->context()); CHECK_EQ(false, async_wrap->persistent().IsWeak()); AfterThreadPoolWork(); } @@ -6247,12 +6246,12 @@ struct RandomBytesJob : public CryptoJob { inline void AfterThreadPoolWork() override { Local arg = ToResult(); - async_wrap->MakeCallback(env->ondone_string(), 1, &arg); + async_wrap->MakeCallback(env()->ondone_string(), 1, &arg); } inline Local ToResult() const { - if (errors.empty()) return Undefined(env->isolate()); - return errors.ToException(env).ToLocalChecked(); + if (errors.empty()) return Undefined(env()->isolate()); + return errors.ToException(env()).ToLocalChecked(); } }; @@ -6304,11 +6303,11 @@ struct PBKDF2Job : public CryptoJob { inline void AfterThreadPoolWork() override { Local arg = ToResult(); - async_wrap->MakeCallback(env->ondone_string(), 1, &arg); + async_wrap->MakeCallback(env()->ondone_string(), 1, &arg); } inline Local ToResult() const { - return Boolean::New(env->isolate(), success.FromJust()); + return Boolean::New(env()->isolate(), success.FromJust()); } inline void Cleanse() { @@ -6384,12 +6383,12 @@ struct ScryptJob : public CryptoJob { inline void AfterThreadPoolWork() override { Local arg = ToResult(); - async_wrap->MakeCallback(env->ondone_string(), 1, &arg); + async_wrap->MakeCallback(env()->ondone_string(), 1, &arg); } inline Local ToResult() const { - if (errors.empty()) return Undefined(env->isolate()); - return errors.ToException(env).ToLocalChecked(); + if (errors.empty()) return Undefined(env()->isolate()); + return errors.ToException(env()).ToLocalChecked(); } inline void Cleanse() { @@ -6653,7 +6652,7 @@ class GenerateKeyPairJob : public CryptoJob { inline void AfterThreadPoolWork() override { Local args[3]; ToResult(&args[0], &args[1], &args[2]); - async_wrap->MakeCallback(env->ondone_string(), 3, args); + async_wrap->MakeCallback(env()->ondone_string(), 3, args); } inline void ToResult(Local* err, @@ -6661,14 +6660,14 @@ class GenerateKeyPairJob : public CryptoJob { Local* privkey) { if (pkey_ && EncodeKeys(pubkey, privkey)) { CHECK(errors_.empty()); - *err = Undefined(env->isolate()); + *err = Undefined(env()->isolate()); } else { if (errors_.empty()) errors_.Capture(); CHECK(!errors_.empty()); - *err = errors_.ToException(env).ToLocalChecked(); - *pubkey = Undefined(env->isolate()); - *privkey = Undefined(env->isolate()); + *err = errors_.ToException(env()).ToLocalChecked(); + *pubkey = Undefined(env()->isolate()); + *privkey = Undefined(env()->isolate()); } } @@ -6677,20 +6676,21 @@ class GenerateKeyPairJob : public CryptoJob { if (public_key_encoding_.output_key_object_) { // Note that this has the downside of containing sensitive data of the // private key. - if (!KeyObject::Create(env, kKeyTypePublic, pkey_).ToLocal(pubkey)) + if (!KeyObject::Create(env(), kKeyTypePublic, pkey_).ToLocal(pubkey)) return false; } else { - if (!WritePublicKey(env, pkey_.get(), public_key_encoding_) + if (!WritePublicKey(env(), pkey_.get(), public_key_encoding_) .ToLocal(pubkey)) return false; } // Now do the same for the private key. if (private_key_encoding_.output_key_object_) { - if (!KeyObject::Create(env, kKeyTypePrivate, pkey_).ToLocal(privkey)) + if (!KeyObject::Create(env(), kKeyTypePrivate, pkey_) + .ToLocal(privkey)) return false; } else { - if (!WritePrivateKey(env, pkey_.get(), private_key_encoding_) + if (!WritePrivateKey(env(), pkey_.get(), private_key_encoding_) .ToLocal(privkey)) return false; } diff --git a/src/node_internals.h b/src/node_internals.h index 114498458a9fd6..68b0852e131e95 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -267,6 +267,8 @@ class ThreadPoolWork { virtual void DoThreadPoolWork() = 0; virtual void AfterThreadPoolWork(int status) = 0; + Environment* env() const { return env_; } + private: Environment* env_; uv_work_t work_req_; diff --git a/src/node_zlib.cc b/src/node_zlib.cc index fdcf685caf2e5b..9c734c17d792f7 100644 --- a/src/node_zlib.cc +++ b/src/node_zlib.cc @@ -348,7 +348,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { if (!async) { // sync version - env()->PrintSyncTrace(); + AsyncWrap::env()->PrintSyncTrace(); DoThreadPoolWork(); if (CheckError()) { UpdateWriteResult(); @@ -397,8 +397,9 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { CHECK_EQ(status, 0); - HandleScope handle_scope(env()->isolate()); - Context::Scope context_scope(env()->context()); + Environment* env = AsyncWrap::env(); + HandleScope handle_scope(env->isolate()); + Context::Scope context_scope(env->context()); if (!CheckError()) return; @@ -406,7 +407,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { UpdateWriteResult(); // call the write() cb - Local cb = PersistentToLocal::Default(env()->isolate(), + Local cb = PersistentToLocal::Default(env->isolate(), write_js_callback_); MakeCallback(cb, 0, nullptr); @@ -416,16 +417,17 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { // TODO(addaleax): Switch to modern error system (node_errors.h). void EmitError(const CompressionError& err) { + Environment* env = AsyncWrap::env(); // If you hit this assertion, you forgot to enter the v8::Context first. - CHECK_EQ(env()->context(), env()->isolate()->GetCurrentContext()); + CHECK_EQ(env->context(), env->isolate()->GetCurrentContext()); - HandleScope scope(env()->isolate()); + HandleScope scope(env->isolate()); Local args[3] = { - OneByteString(env()->isolate(), err.message), - Integer::New(env()->isolate(), err.err), - OneByteString(env()->isolate(), err.code) + OneByteString(env->isolate(), err.message), + Integer::New(env->isolate(), err.err), + OneByteString(env->isolate(), err.code) }; - MakeCallback(env()->onerror_string(), arraysize(args), args); + MakeCallback(env->onerror_string(), arraysize(args), args); // no hope of rescue. write_in_progress_ = false; @@ -454,7 +456,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { void InitStream(uint32_t* write_result, Local write_js_callback) { write_result_ = write_result; - write_js_callback_.Reset(env()->isolate(), write_js_callback); + write_js_callback_.Reset(AsyncWrap::env()->isolate(), write_js_callback); init_done_ = true; } @@ -500,7 +502,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { if (report == 0) return; CHECK_IMPLIES(report < 0, zlib_memory_ >= static_cast(-report)); zlib_memory_ += report; - env()->isolate()->AdjustAmountOfExternalAllocatedMemory(report); + AsyncWrap::env()->isolate()->AdjustAmountOfExternalAllocatedMemory(report); } struct AllocScope {