src: remove StreamBase::kFlagHasWritev

Since libuv 1.21.0, pipes on Windows support `writev` on the libuv side. This allows for some simplification, and makes the `StreamBase` API more uniform (multi-buffer `Write()` is always supported now, including when used by other non-JS consumers like HTTP/2). PR-URL: #21527 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Jun 30, 2018 · 64a3fad · 64a3fad
1 parent a078521
commit 64a3fad
Show file tree

Hide file tree

Showing 12 changed files with 63 additions and 30 deletions.
diff --git a/lib/net.js b/lib/net.js
@@ -210,10 +210,6 @@ function initSocketHandle(self) {
     self._handle.owner = self;
     self._handle.onread = onread;
     self[async_id_symbol] = getNewAsyncId(self._handle);
-
-    // If handle doesn't support writev - neither do we
-    if (!self._handle.writev)
-      self._writev = null;
   }
 }
 

diff --git a/src/js_stream.cc b/src/js_stream.cc
@@ -208,7 +208,7 @@ void JSStream::Initialize(Local<Object> target,
   env->SetProtoMethod(t, "readBuffer", ReadBuffer);
   env->SetProtoMethod(t, "emitEOF", EmitEOF);
 
-  StreamBase::AddMethods<JSStream>(env, t, StreamBase::kFlagHasWritev);
+  StreamBase::AddMethods<JSStream>(env, t);
   target->Set(jsStreamString, t->GetFunction());
 }
 

diff --git a/src/node_file.cc b/src/node_file.cc
@@ -1997,7 +1997,7 @@ void Initialize(Local<Object> target,
   Local<String> handleString =
        FIXED_ONE_BYTE_STRING(env->isolate(), "FileHandle");
   fd->SetClassName(handleString);
-  StreamBase::AddMethods<FileHandle>(env, fd, StreamBase::kFlagNone);
+  StreamBase::AddMethods<FileHandle>(env, fd);
   target->Set(context, handleString, fd->GetFunction()).FromJust();
   env->set_fd_constructor_template(fdt);
 

diff --git a/src/node_http2.cc b/src/node_http2.cc
@@ -2840,7 +2840,7 @@ void Initialize(Local<Object> target,
   env->SetProtoMethod(stream, "rstStream", Http2Stream::RstStream);
   env->SetProtoMethod(stream, "refreshState", Http2Stream::RefreshState);
   AsyncWrap::AddWrapMethods(env, stream);
-  StreamBase::AddMethods<Http2Stream>(env, stream, StreamBase::kFlagHasWritev);
+  StreamBase::AddMethods<Http2Stream>(env, stream);
   Local<ObjectTemplate> streamt = stream->InstanceTemplate();
   streamt->SetInternalFieldCount(1);
   env->set_http2stream_constructor_template(streamt);

diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc
@@ -83,11 +83,7 @@ void PipeWrap::Initialize(Local<Object> target,
   env->SetProtoMethod(t, "ref", HandleWrap::Ref);
   env->SetProtoMethod(t, "hasRef", HandleWrap::HasRef);
 
-#ifdef _WIN32
   LibuvStreamWrap::AddMethods(env, t);
-#else
-  LibuvStreamWrap::AddMethods(env, t, StreamBase::kFlagHasWritev);
-#endif
 
   env->SetProtoMethod(t, "bind", Bind);
   env->SetProtoMethod(t, "listen", Listen);

diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h
@@ -266,9 +266,7 @@ inline WriteWrap* StreamBase::CreateWriteWrap(
 }
 
 template <class Base>
-void StreamBase::AddMethods(Environment* env,
-                            Local<FunctionTemplate> t,
-                            int flags) {
+void StreamBase::AddMethods(Environment* env, Local<FunctionTemplate> t) {
   HandleScope scope(env->isolate());
 
   enum PropertyAttribute attributes =
@@ -325,8 +323,7 @@ void StreamBase::AddMethods(Environment* env,
   env->SetProtoMethod(t, "readStart", JSMethod<Base, &StreamBase::ReadStartJS>);
   env->SetProtoMethod(t, "readStop", JSMethod<Base, &StreamBase::ReadStopJS>);
   env->SetProtoMethod(t, "shutdown", JSMethod<Base, &StreamBase::Shutdown>);
-  if ((flags & kFlagHasWritev) != 0)
-    env->SetProtoMethod(t, "writev", JSMethod<Base, &StreamBase::Writev>);
+  env->SetProtoMethod(t, "writev", JSMethod<Base, &StreamBase::Writev>);
   env->SetProtoMethod(t,
                       "writeBuffer",
                       JSMethod<Base, &StreamBase::WriteBuffer>);

diff --git a/src/stream_base.h b/src/stream_base.h
@@ -255,15 +255,9 @@ class StreamResource {
 
 class StreamBase : public StreamResource {
  public:
-  enum Flags {
-    kFlagNone = 0x0,
-    kFlagHasWritev = 0x1
-  };
-
   template <class Base>
   static inline void AddMethods(Environment* env,
-                                v8::Local<v8::FunctionTemplate> target,
-                                int flags = kFlagNone);
+                                v8::Local<v8::FunctionTemplate> target);
 
   virtual bool IsAlive() = 0;
   virtual bool IsClosing() = 0;

diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc
@@ -97,8 +97,7 @@ LibuvStreamWrap::LibuvStreamWrap(Environment* env,
 
 
 void LibuvStreamWrap::AddMethods(Environment* env,
-                                 v8::Local<v8::FunctionTemplate> target,
-                                 int flags) {
+                                 v8::Local<v8::FunctionTemplate> target) {
   Local<FunctionTemplate> get_write_queue_size =
       FunctionTemplate::New(env->isolate(),
                             GetWriteQueueSize,
@@ -110,7 +109,7 @@ void LibuvStreamWrap::AddMethods(Environment* env,
       Local<FunctionTemplate>(),
       static_cast<PropertyAttribute>(ReadOnly | DontDelete));
   env->SetProtoMethod(target, "setBlocking", SetBlocking);
-  StreamBase::AddMethods<LibuvStreamWrap>(env, target, flags);
+  StreamBase::AddMethods<LibuvStreamWrap>(env, target);
 }
 
 

diff --git a/src/stream_wrap.h b/src/stream_wrap.h
@@ -85,8 +85,7 @@ class LibuvStreamWrap : public HandleWrap, public StreamBase {
   AsyncWrap* GetAsyncWrap() override;
 
   static void AddMethods(Environment* env,
-                         v8::Local<v8::FunctionTemplate> target,
-                         int flags = StreamBase::kFlagNone);
+                         v8::Local<v8::FunctionTemplate> target);
 
  protected:
   inline void set_fd(int fd) {

diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc
@@ -93,7 +93,7 @@ void TCPWrap::Initialize(Local<Object> target,
   env->SetProtoMethod(t, "unref", HandleWrap::Unref);
   env->SetProtoMethod(t, "hasRef", HandleWrap::HasRef);
 
-  LibuvStreamWrap::AddMethods(env, t, StreamBase::kFlagHasWritev);
+  LibuvStreamWrap::AddMethods(env, t);
 
   env->SetProtoMethod(t, "open", Open);
   env->SetProtoMethod(t, "bind", Bind);

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
@@ -895,7 +895,7 @@ void TLSWrap::Initialize(Local<Object> target,
   env->SetProtoMethod(t, "destroySSL", DestroySSL);
   env->SetProtoMethod(t, "enableCertCb", EnableCertCb);
 
-  StreamBase::AddMethods<TLSWrap>(env, t, StreamBase::kFlagHasWritev);
+  StreamBase::AddMethods<TLSWrap>(env, t);
   SSLWrap<TLSWrap>::AddMethods(env, t);
 
   env->SetProtoMethod(t, "getServername", GetServername);

diff --git a/test/parallel/test-http2-pipe-named-pipe.js b/test/parallel/test-http2-pipe-named-pipe.js
@@ -0,0 +1,52 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const fixtures = require('../common/fixtures');
+const assert = require('assert');
+const http2 = require('http2');
+const fs = require('fs');
+const net = require('net');
+const path = require('path');
+
+// HTTP/2 servers can listen on a named pipe.
+
+const tmpdir = require('../common/tmpdir');
+tmpdir.refresh();
+const loc = fixtures.path('url-tests.js');
+const fn = path.join(tmpdir.path, 'http2-url-tests.js');
+
+const server = http2.createServer();
+
+server.on('stream', common.mustCall((stream) => {
+  const dest = stream.pipe(fs.createWriteStream(fn));
+
+  dest.on('finish', () => {
+    assert.strictEqual(fs.readFileSync(loc).length,
+                       fs.readFileSync(fn).length);
+  });
+  stream.respond();
+  stream.end();
+}));
+
+server.listen(common.PIPE, common.mustCall(() => {
+  const client = http2.connect('http://localhost', {
+    createConnection(url) {
+      return net.connect(server.address());
+    }
+  });
+
+  const req = client.request({ ':method': 'POST' });
+  req.on('response', common.mustCall());
+  req.resume();
+
+  req.on('close', common.mustCall(() => {
+    server.close();
+    client.close();
+  }));
+
+  const str = fs.createReadStream(loc);
+  str.on('end', common.mustCall());
+  str.pipe(req);
+}));