Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib,src: remove usage of _externalStream #26510

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -390,13 +390,11 @@ function connectionListenerInternal(server, socket) {
socket.on = socketOnWrap;

// We only consume the socket if it has never been consumed before.
if (socket._handle) {
var external = socket._handle._externalStream;
if (!socket._handle._consumed && external) {
parser._consumed = true;
socket._handle._consumed = true;
parser.consume(external);
}
if (socket._handle && socket._handle.isStreamBase &&
!socket._handle._consumed) {
parser._consumed = true;
socket._handle._consumed = true;
parser.consume(socket._handle);
}
parser[kOnExecute] =
onParserExecute.bind(undefined, server, socket, parser, state);
Expand Down
8 changes: 2 additions & 6 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -444,14 +444,10 @@ TLSSocket.prototype._wrapHandle = function(wrap) {
const context = options.secureContext ||
options.credentials ||
tls.createSecureContext(options);
const externalStream = handle._externalStream;
assert(typeof externalStream === 'object',
'handle must be a LibuvStreamWrap');
assert(handle.isStreamBase, 'handle must be a StreamBase');
assert(context.context instanceof NativeSecureContext,
'context.context must be a NativeSecureContext');
const res = tls_wrap.wrap(externalStream,
context.context,
!!options.isServer);
const res = tls_wrap.wrap(handle, context.context, !!options.isServer);
res._parent = handle; // C++ "wrap" object: TCPWrap, JSStream, ...
res._parentWrap = wrap; // JS object: net.Socket, JSStreamSocket, ...
res._secureContext = context;
Expand Down
7 changes: 3 additions & 4 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@ function setupHandle(socket, type, options) {

if (typeof options.selectPadding === 'function')
this[kSelectPadding] = options.selectPadding;
handle.consume(socket._handle._externalStream);
handle.consume(socket._handle);

this[kHandle] = handle;

Expand Down Expand Up @@ -934,7 +934,7 @@ class Http2Session extends EventEmitter {
constructor(type, options, socket) {
super();

if (!socket._handle || !socket._handle._externalStream) {
if (!socket._handle || !socket._handle.isStreamBase) {
socket = new JSStreamSocket(socket);
}

Expand Down Expand Up @@ -2094,8 +2094,7 @@ function startFilePipe(self, fd, offset, length) {
handle.onread = onPipedFileHandleRead;
handle.stream = self;

const pipe = new StreamPipe(handle._externalStream,
self[kHandle]._externalStream);
const pipe = new StreamPipe(handle, self[kHandle]);
pipe.onunpipe = onFileUnpipe;
pipe.start();

Expand Down
8 changes: 4 additions & 4 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1837,8 +1837,8 @@ bool Http2Session::HasWritesOnSocketForStream(Http2Stream* stream) {
// (typically a net.Socket or tls.TLSSocket). The lifecycle of the two is
// tightly coupled with all data transfer between the two happening at the
// C++ layer via the StreamBase API.
void Http2Session::Consume(Local<External> external) {
StreamBase* stream = static_cast<StreamBase*>(external->Value());
void Http2Session::Consume(Local<Object> stream_obj) {
StreamBase* stream = StreamBase::FromObject(stream_obj);
stream->PushStreamListener(this);
Debug(this, "i/o stream consumed");
}
Expand Down Expand Up @@ -2429,8 +2429,8 @@ void Http2Session::New(const FunctionCallbackInfo<Value>& args) {
void Http2Session::Consume(const FunctionCallbackInfo<Value>& args) {
Http2Session* session;
ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder());
CHECK(args[0]->IsExternal());
session->Consume(args[0].As<External>());
CHECK(args[0]->IsObject());
session->Consume(args[0].As<Object>());
}

// Destroys the Http2Session instance and renders it unusable
Expand Down
2 changes: 1 addition & 1 deletion src/node_http2.h
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ class Http2Session : public AsyncWrap, public StreamListener {

void Close(uint32_t code = NGHTTP2_NO_ERROR,
bool socket_closed = false);
void Consume(Local<External> external);
void Consume(Local<Object> stream);
void Goaway(uint32_t code, int32_t lastStreamID,
const uint8_t* data, size_t len);
void AltSvc(int32_t id,
Expand Down
5 changes: 2 additions & 3 deletions src/node_http_parser_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -556,9 +556,8 @@ class Parser : public AsyncWrap, public StreamListener {
static void Consume(const FunctionCallbackInfo<Value>& args) {
Parser* parser;
ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder());
CHECK(args[0]->IsExternal());
Local<External> stream_obj = args[0].As<External>();
StreamBase* stream = static_cast<StreamBase*>(stream_obj->Value());
CHECK(args[0]->IsObject());
StreamBase* stream = StreamBase::FromObject(args[0].As<Object>());
CHECK_NOT_NULL(stream);
stream->PushStreamListener(parser);
}
Expand Down
1 change: 0 additions & 1 deletion src/stream_base-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
namespace node {

using v8::Signature;
using v8::External;
using v8::FunctionCallbackInfo;
using v8::FunctionTemplate;
using v8::HandleScope;
Expand Down
4 changes: 4 additions & 0 deletions src/stream_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace node {
using v8::Array;
using v8::ArrayBuffer;
using v8::Context;
using v8::External;
using v8::FunctionCallbackInfo;
using v8::HandleScope;
using v8::Integer;
Expand Down Expand Up @@ -368,6 +369,9 @@ void StreamBase::AddMethods(Environment* env, Local<FunctionTemplate> t) {
t, "writeUcs2String", JSMethod<&StreamBase::WriteString<UCS2>>);
env->SetProtoMethod(
t, "writeLatin1String", JSMethod<&StreamBase::WriteString<LATIN1>>);
t->PrototypeTemplate()->Set(FIXED_ONE_BYTE_STRING(env->isolate(),
"isStreamBase"),
True(env->isolate()));
}

void StreamBase::GetFD(const FunctionCallbackInfo<Value>& args) {
Expand Down
9 changes: 4 additions & 5 deletions src/stream_pipe.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
#include "node_buffer.h"

using v8::Context;
using v8::External;
using v8::Function;
using v8::FunctionCallbackInfo;
using v8::FunctionTemplate;
Expand Down Expand Up @@ -226,10 +225,10 @@ void StreamPipe::WritableListener::OnStreamRead(ssize_t nread,

void StreamPipe::New(const FunctionCallbackInfo<Value>& args) {
CHECK(args.IsConstructCall());
CHECK(args[0]->IsExternal());
CHECK(args[1]->IsExternal());
auto source = static_cast<StreamBase*>(args[0].As<External>()->Value());
auto sink = static_cast<StreamBase*>(args[1].As<External>()->Value());
CHECK(args[0]->IsObject());
CHECK(args[1]->IsObject());
StreamBase* source = StreamBase::FromObject(args[0].As<Object>());
StreamBase* sink = StreamBase::FromObject(args[1].As<Object>());

new StreamPipe(source, sink, args.This());
}
Expand Down
3 changes: 1 addition & 2 deletions src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,11 @@ void TLSWrap::Wrap(const FunctionCallbackInfo<Value>& args) {
CHECK(args[1]->IsObject());
CHECK(args[2]->IsBoolean());

Local<External> stream_obj = args[0].As<External>();
Local<Object> sc = args[1].As<Object>();
Kind kind = args[2]->IsTrue() ? SSLWrap<TLSWrap>::kServer :
SSLWrap<TLSWrap>::kClient;

StreamBase* stream = static_cast<StreamBase*>(stream_obj->Value());
StreamBase* stream = StreamBase::FromObject(args[0].As<Object>());
CHECK_NOT_NULL(stream);

Local<Object> obj;
Expand Down
3 changes: 1 addition & 2 deletions test/sequential/test-async-wrap-getasyncid.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,7 @@ if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check

// TLSWrap is exposed, but needs to be instantiated via tls_wrap.wrap().
const tls_wrap = internalBinding('tls_wrap');
testInitialized(
tls_wrap.wrap(tcp._externalStream, credentials.context, true), 'TLSWrap');
testInitialized(tls_wrap.wrap(tcp, credentials.context, true), 'TLSWrap');
}

{
Expand Down
4 changes: 2 additions & 2 deletions test/sequential/test-http-regr-gh-2928.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ function execAndClose() {
throw e;
});

parser.consume(socket._handle._externalStream);
parser.consume(socket._handle);

parser.onIncoming = function onIncoming() {
process.stdout.write('+');
gotResponses++;
parser.unconsume(socket._handle._externalStream);
parser.unconsume();
httpCommon.freeParser(parser);
socket.destroy();
setImmediate(execAndClose);
Expand Down