src: StreamBase::stream_env() -> StreamBase::env()

Rename `StreamBase::stream_env()` to `StreamBase::env()` and work around the diamond problem by adding `env()` getters to several classes. Refs: nodejs#31554 (comment)
bnoordhuis · Jan 31, 2020 · 47c4eac · 47c4eac
1 parent 94935b2
commit 47c4eac
Show file tree

Hide file tree

Showing 9 changed files with 63 additions and 65 deletions.
diff --git a/src/js_stream.h b/src/js_stream.h
@@ -32,6 +32,8 @@ class JSStream : public AsyncWrap, public StreamBase {
   SET_MEMORY_INFO_NAME(JSStream)
   SET_SELF_SIZE(JSStream)
 
+  Environment* env() const { return AsyncWrap::env(); }
+
  protected:
   JSStream(Environment* env, v8::Local<v8::Object> obj);
 

diff --git a/src/node_file.h b/src/node_file.h
@@ -205,6 +205,8 @@ class FileHandle final : public AsyncWrap, public StreamBase {
 
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
 
+  Environment* env() const { return AsyncWrap::env(); }
+
   int GetFD() override { return fd_; }
 
   // Will asynchronously close the FD and return a Promise that will

diff --git a/src/node_http2.h b/src/node_http2.h
@@ -458,6 +458,7 @@ class Http2Stream : public AsyncWrap,
 
   nghttp2_stream* operator*();
 
+  Environment* env() const { return AsyncWrap::env(); }
   Http2Session* session() { return session_.get(); }
   const Http2Session* session() const { return session_.get(); }
 

diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h
@@ -140,19 +140,17 @@ inline StreamBase::StreamBase(Environment* env) : env_(env) {
   PushStreamListener(&default_listener_);
 }
 
-inline Environment* StreamBase::stream_env() const {
+inline Environment* StreamBase::env() const {
   return env_;
 }
 
 inline int StreamBase::Shutdown(v8::Local<v8::Object> req_wrap_obj) {
-  Environment* env = stream_env();
-
-  HandleScope handle_scope(env->isolate());
+  HandleScope handle_scope(env()->isolate());
 
   if (req_wrap_obj.IsEmpty()) {
-    if (!env->shutdown_wrap_template()
-             ->NewInstance(env->context())
-             .ToLocal(&req_wrap_obj)) {
+    if (!env()->shutdown_wrap_template()
+               ->NewInstance(env()->context())
+               .ToLocal(&req_wrap_obj)) {
       return UV_EBUSY;
     }
     StreamReq::ResetObject(req_wrap_obj);
@@ -169,8 +167,8 @@ inline int StreamBase::Shutdown(v8::Local<v8::Object> req_wrap_obj) {
   const char* msg = Error();
   if (msg != nullptr) {
     req_wrap_obj->Set(
-        env->context(),
-        env->error_string(), OneByteString(env->isolate(), msg)).Check();
+        env()->context(),
+        env()->error_string(), OneByteString(env()->isolate(), msg)).Check();
     ClearError();
   }
 
@@ -182,7 +180,6 @@ inline StreamWriteResult StreamBase::Write(
     size_t count,
     uv_stream_t* send_handle,
     v8::Local<v8::Object> req_wrap_obj) {
-  Environment* env = stream_env();
   int err;
 
   size_t total_bytes = 0;
@@ -197,12 +194,12 @@ inline StreamWriteResult StreamBase::Write(
     }
   }
 
-  HandleScope handle_scope(env->isolate());
+  HandleScope handle_scope(env()->isolate());
 
   if (req_wrap_obj.IsEmpty()) {
-    if (!env->write_wrap_template()
-             ->NewInstance(env->context())
-             .ToLocal(&req_wrap_obj)) {
+    if (!env()->write_wrap_template()
+               ->NewInstance(env()->context())
+               .ToLocal(&req_wrap_obj)) {
       return StreamWriteResult { false, UV_EBUSY, nullptr, 0 };
     }
     StreamReq::ResetObject(req_wrap_obj);
@@ -221,9 +218,9 @@ inline StreamWriteResult StreamBase::Write(
 
   const char* msg = Error();
   if (msg != nullptr) {
-    req_wrap_obj->Set(env->context(),
-                      env->error_string(),
-                      OneByteString(env->isolate(), msg)).Check();
+    req_wrap_obj->Set(env()->context(),
+                      env()->error_string(),
+                      OneByteString(env()->isolate(), msg)).Check();
     ClearError();
   }
 
@@ -235,9 +232,7 @@ SimpleShutdownWrap<OtherBase>::SimpleShutdownWrap(
     StreamBase* stream,
     v8::Local<v8::Object> req_wrap_obj)
   : ShutdownWrap(stream, req_wrap_obj),
-    OtherBase(stream->stream_env(),
-              req_wrap_obj,
-              AsyncWrap::PROVIDER_SHUTDOWNWRAP) {
+    OtherBase(stream->env(), req_wrap_obj, AsyncWrap::PROVIDER_SHUTDOWNWRAP) {
 }
 
 inline ShutdownWrap* StreamBase::CreateShutdownWrap(
@@ -250,9 +245,7 @@ SimpleWriteWrap<OtherBase>::SimpleWriteWrap(
     StreamBase* stream,
     v8::Local<v8::Object> req_wrap_obj)
   : WriteWrap(stream, req_wrap_obj),
-    OtherBase(stream->stream_env(),
-              req_wrap_obj,
-              AsyncWrap::PROVIDER_WRITEWRAP) {
+    OtherBase(stream->env(), req_wrap_obj, AsyncWrap::PROVIDER_WRITEWRAP) {
 }
 
 inline WriteWrap* StreamBase::CreateWriteWrap(

diff --git a/src/stream_base.cc b/src/stream_base.cc
@@ -72,8 +72,6 @@ void StreamBase::SetWriteResult(const StreamWriteResult& res) {
 }
 
 int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-
   CHECK(args[0]->IsObject());
   CHECK(args[1]->IsArray());
 
@@ -95,21 +93,23 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
   if (!all_buffers) {
     // Determine storage size first
     for (size_t i = 0; i < count; i++) {
-      Local<Value> chunk = chunks->Get(env->context(), i * 2).ToLocalChecked();
+      Local<Value> chunk =
+          chunks->Get(env()->context(), i * 2).ToLocalChecked();
 
       if (Buffer::HasInstance(chunk))
         continue;
         // Buffer chunk, no additional storage required
 
       // String chunk
-      Local<String> string = chunk->ToString(env->context()).ToLocalChecked();
-      enum encoding encoding = ParseEncoding(env->isolate(),
-          chunks->Get(env->context(), i * 2 + 1).ToLocalChecked());
+      Local<String> string = chunk->ToString(env()->context()).ToLocalChecked();
+      enum encoding encoding = ParseEncoding(env()->isolate(),
+          chunks->Get(env()->context(), i * 2 + 1).ToLocalChecked());
       size_t chunk_size;
       if (encoding == UTF8 && string->Length() > 65535 &&
-          !StringBytes::Size(env->isolate(), string, encoding).To(&chunk_size))
+          !StringBytes::Size(env()->isolate(), string, encoding)
+          .To(&chunk_size))
         return 0;
-      else if (!StringBytes::StorageSize(env->isolate(), string, encoding)
+      else if (!StringBytes::StorageSize(env()->isolate(), string, encoding)
                     .To(&chunk_size))
         return 0;
       storage_size += chunk_size;
@@ -119,20 +119,21 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
       return UV_ENOBUFS;
   } else {
     for (size_t i = 0; i < count; i++) {
-      Local<Value> chunk = chunks->Get(env->context(), i).ToLocalChecked();
+      Local<Value> chunk = chunks->Get(env()->context(), i).ToLocalChecked();
       bufs[i].base = Buffer::Data(chunk);
       bufs[i].len = Buffer::Length(chunk);
     }
   }
 
   AllocatedBuffer storage;
   if (storage_size > 0)
-    storage = env->AllocateManaged(storage_size);
+    storage = env()->AllocateManaged(storage_size);
 
   offset = 0;
   if (!all_buffers) {
     for (size_t i = 0; i < count; i++) {
-      Local<Value> chunk = chunks->Get(env->context(), i * 2).ToLocalChecked();
+      Local<Value> chunk =
+          chunks->Get(env()->context(), i * 2).ToLocalChecked();
 
       // Write buffer
       if (Buffer::HasInstance(chunk)) {
@@ -146,10 +147,10 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
       char* str_storage = storage.data() + offset;
       size_t str_size = storage.size() - offset;
 
-      Local<String> string = chunk->ToString(env->context()).ToLocalChecked();
-      enum encoding encoding = ParseEncoding(env->isolate(),
-          chunks->Get(env->context(), i * 2 + 1).ToLocalChecked());
-      str_size = StringBytes::Write(env->isolate(),
+      Local<String> string = chunk->ToString(env()->context()).ToLocalChecked();
+      enum encoding encoding = ParseEncoding(env()->isolate(),
+          chunks->Get(env()->context(), i * 2 + 1).ToLocalChecked());
+      str_size = StringBytes::Write(env()->isolate(),
                                     str_storage,
                                     str_size,
                                     string,
@@ -172,10 +173,8 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
 int StreamBase::WriteBuffer(const FunctionCallbackInfo<Value>& args) {
   CHECK(args[0]->IsObject());
 
-  Environment* env = Environment::GetCurrent(args);
-
   if (!args[1]->IsUint8Array()) {
-    node::THROW_ERR_INVALID_ARG_TYPE(env, "Second argument must be a buffer");
+    node::THROW_ERR_INVALID_ARG_TYPE(env(), "Second argument must be a buffer");
     return 0;
   }
 
@@ -194,8 +193,8 @@ int StreamBase::WriteBuffer(const FunctionCallbackInfo<Value>& args) {
     send_handle = reinterpret_cast<uv_stream_t*>(wrap->GetHandle());
     // Reference LibuvStreamWrap instance to prevent it from being garbage
     // collected before `AfterWrite` is called.
-    req_wrap_obj->Set(env->context(),
-                      env->handle_string(),
+    req_wrap_obj->Set(env()->context(),
+                      env()->handle_string(),
                       send_handle_obj).Check();
   }
 
@@ -208,7 +207,6 @@ int StreamBase::WriteBuffer(const FunctionCallbackInfo<Value>& args) {
 
 template <enum encoding enc>
 int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
   CHECK(args[0]->IsObject());
   CHECK(args[1]->IsString());
 
@@ -223,9 +221,9 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) {
   // computing their actual size, rather than tripling the storage.
   size_t storage_size;
   if (enc == UTF8 && string->Length() > 65535 &&
-      !StringBytes::Size(env->isolate(), string, enc).To(&storage_size))
+      !StringBytes::Size(env()->isolate(), string, enc).To(&storage_size))
     return 0;
-  else if (!StringBytes::StorageSize(env->isolate(), string, enc)
+  else if (!StringBytes::StorageSize(env()->isolate(), string, enc)
                 .To(&storage_size))
     return 0;
 
@@ -241,7 +239,7 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) {
   bool try_write = storage_size <= sizeof(stack_storage) &&
                    (!IsIPCPipe() || send_handle_obj.IsEmpty());
   if (try_write) {
-    data_size = StringBytes::Write(env->isolate(),
+    data_size = StringBytes::Write(env()->isolate(),
                                    stack_storage,
                                    storage_size,
                                    string,
@@ -271,13 +269,13 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) {
 
   if (try_write) {
     // Copy partial data
-    data = env->AllocateManaged(buf.len);
+    data = env()->AllocateManaged(buf.len);
     memcpy(data.data(), buf.base, buf.len);
     data_size = buf.len;
   } else {
     // Write it
-    data = env->AllocateManaged(storage_size);
-    data_size = StringBytes::Write(env->isolate(),
+    data = env()->AllocateManaged(storage_size);
+    data_size = StringBytes::Write(env()->isolate(),
                                    data.data(),
                                    storage_size,
                                    string,
@@ -296,8 +294,8 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) {
     send_handle = reinterpret_cast<uv_stream_t*>(wrap->GetHandle());
     // Reference LibuvStreamWrap instance to prevent it from being garbage
     // collected before `AfterWrite` is called.
-    req_wrap_obj->Set(env->context(),
-                      env->handle_string(),
+    req_wrap_obj->Set(env()->context(),
+                      env()->handle_string(),
                       send_handle_obj).Check();
   }
 
@@ -317,8 +315,6 @@ MaybeLocal<Value> StreamBase::CallJSOnreadMethod(ssize_t nread,
                                                  Local<ArrayBuffer> ab,
                                                  size_t offset,
                                                  StreamBaseJSChecks checks) {
-  Environment* env = env_;
-
   DCHECK_EQ(static_cast<int32_t>(nread), nread);
   DCHECK_LE(offset, INT32_MAX);
 
@@ -331,11 +327,11 @@ MaybeLocal<Value> StreamBase::CallJSOnreadMethod(ssize_t nread,
     }
   }
 
-  env->stream_base_state()[kReadBytesOrError] = nread;
-  env->stream_base_state()[kArrayBufferOffset] = offset;
+  env()->stream_base_state()[kReadBytesOrError] = nread;
+  env()->stream_base_state()[kArrayBufferOffset] = offset;
 
   Local<Value> argv[] = {
-    ab.IsEmpty() ? Undefined(env->isolate()).As<Value>() : ab.As<Value>()
+    ab.IsEmpty() ? Undefined(env()->isolate()).As<Value>() : ab.As<Value>()
   };
 
   AsyncWrap* wrap = GetAsyncWrap();
@@ -476,14 +472,14 @@ void StreamResource::ClearError() {
 
 uv_buf_t EmitToJSStreamListener::OnStreamAlloc(size_t suggested_size) {
   CHECK_NOT_NULL(stream_);
-  Environment* env = static_cast<StreamBase*>(stream_)->stream_env();
+  Environment* env = static_cast<StreamBase*>(stream_)->env();
   return env->AllocateManaged(suggested_size).release();
 }
 
 void EmitToJSStreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
   CHECK_NOT_NULL(stream_);
   StreamBase* stream = static_cast<StreamBase*>(stream_);
-  Environment* env = stream->stream_env();
+  Environment* env = stream->env();
   HandleScope handle_scope(env->isolate());
   Context::Scope context_scope(env->context());
   AllocatedBuffer buf(env, buf_);
@@ -511,7 +507,7 @@ void CustomBufferJSListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) {
   CHECK_EQ(buf.base, buffer_.base);
 
   StreamBase* stream = static_cast<StreamBase*>(stream_);
-  Environment* env = stream->stream_env();
+  Environment* env = stream->env();
   HandleScope handle_scope(env->isolate());
   Context::Scope context_scope(env->context());
 
@@ -530,7 +526,7 @@ void CustomBufferJSListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) {
 void ReportWritesToJSStreamListener::OnStreamAfterReqFinished(
     StreamReq* req_wrap, int status) {
   StreamBase* stream = static_cast<StreamBase*>(stream_);
-  Environment* env = stream->stream_env();
+  Environment* env = stream->env();
   AsyncWrap* async_wrap = req_wrap->GetAsyncWrap();
   HandleScope handle_scope(env->isolate());
   Context::Scope context_scope(env->context());

diff --git a/src/stream_base.h b/src/stream_base.h
@@ -296,9 +296,7 @@ class StreamBase : public StreamResource {
       size_t offset = 0,
       StreamBaseJSChecks checks = DONT_SKIP_NREAD_CHECKS);
 
-  // This is named `stream_env` to avoid name clashes, because a lot of
-  // subclasses are also `BaseObject`s.
-  Environment* stream_env() const;
+  Environment* env() const;
 
   // Shut down the current stream. This request can use an existing
   // ShutdownWrap object (that was created in JS), or a new one will be created.

diff --git a/src/stream_pipe.cc b/src/stream_pipe.cc
@@ -16,7 +16,7 @@ namespace node {
 StreamPipe::StreamPipe(StreamBase* source,
                        StreamBase* sink,
                        Local<Object> obj)
-    : AsyncWrap(source->stream_env(), obj, AsyncWrap::PROVIDER_STREAMPIPE) {
+    : AsyncWrap(source->env(), obj, AsyncWrap::PROVIDER_STREAMPIPE) {
   MakeWeak();
 
   CHECK_NOT_NULL(sink);

diff --git a/src/stream_wrap.h b/src/stream_wrap.h
@@ -56,6 +56,10 @@ class LibuvStreamWrap : public HandleWrap, public StreamBase {
               size_t count,
               uv_stream_t* send_handle) override;
 
+  inline Environment* env() const {
+    return AsyncWrap::env();
+  }
+
   inline uv_stream_t* stream() const {
     return stream_;
   }

diff --git a/src/tls_wrap.h b/src/tls_wrap.h
@@ -56,6 +56,8 @@ class TLSWrap : public AsyncWrap,
                          v8::Local<v8::Context> context,
                          void* priv);
 
+  Environment* env() const { return AsyncWrap::env(); }
+
   // Implement StreamBase:
   bool IsAlive() override;
   bool IsClosing() override;