Skip to content

Commit

Permalink
src: remove StreamBase::kFlagHasWritev
Browse files Browse the repository at this point in the history
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 <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax authored and targos committed Jul 3, 2018
1 parent a8a7575 commit 0550a58
Show file tree
Hide file tree
Showing 12 changed files with 63 additions and 30 deletions.
4 changes: 0 additions & 4 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/js_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand Down
2 changes: 1 addition & 1 deletion src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 0 additions & 4 deletions src/pipe_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 2 additions & 5 deletions src/stream_base-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -324,8 +322,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>);
Expand Down
8 changes: 1 addition & 7 deletions src/stream_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions src/stream_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
}


Expand Down
3 changes: 1 addition & 2 deletions src/stream_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/tcp_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
52 changes: 52 additions & 0 deletions test/parallel/test-http2-pipe-named-pipe.js
Original file line number Diff line number Diff line change
@@ -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);
}));

0 comments on commit 0550a58

Please sign in to comment.