Skip to content

Remove StreamBase templating #25142

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 8 commits into from
Closed

Remove StreamBase templating #25142

wants to merge 8 commits into from

Conversation

@maclover7
Copy link
Contributor

@maclover7 maclover7 commented Dec 19, 2018 •
edited
Loading

First attempt at trying to remove the templating in shared StreamBase methods. This approach casts directly to StreamBase via an internal field.

Unfortunately, I keep hitting linker errors when trying to run make -j8, and would appreciate any help in figuring out where they're coming from:

duplicate symbol __ZN4node10StreamBase15GetBytesWrittenERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(js_stream.o)
duplicate symbol __ZN4node10StreamBase11GetExternalERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(js_stream.o)
duplicate symbol __ZN4node10StreamBase12GetBytesReadERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(js_stream.o)
duplicate symbol __ZN4node10StreamBase5GetFDERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(js_stream.o)
duplicate symbol __ZN4node10StreamBase15GetBytesWrittenERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(node_file.o)
duplicate symbol __ZN4node10StreamBase11GetExternalERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(node_file.o)
duplicate symbol __ZN4node10StreamBase12GetBytesReadERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(node_file.o)
duplicate symbol __ZN4node10StreamBase5GetFDERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(node_file.o)
duplicate symbol __ZN4node10StreamBase15GetBytesWrittenERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(node_http_parser_llhttp.o)
duplicate symbol __ZN4node10StreamBase11GetExternalERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(node_http_parser_llhttp.o)
duplicate symbol __ZN4node10StreamBase12GetBytesReadERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(node_http_parser_llhttp.o)
duplicate symbol __ZN4node10StreamBase5GetFDERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(node_http_parser_llhttp.o)
duplicate symbol __ZN4node10StreamBase15GetBytesWrittenERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(node_http_parser_traditional.o)
duplicate symbol __ZN4node10StreamBase11GetExternalERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(node_http_parser_traditional.o)
duplicate symbol __ZN4node10StreamBase12GetBytesReadERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(node_http_parser_traditional.o)
duplicate symbol __ZN4node10StreamBase5GetFDERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(node_http_parser_traditional.o)
duplicate symbol __ZN4node10StreamBase15GetBytesWrittenERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(node_http2.o)
duplicate symbol __ZN4node10StreamBase11GetExternalERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(node_http2.o)
duplicate symbol __ZN4node10StreamBase12GetBytesReadERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(node_http2.o)
duplicate symbol __ZN4node10StreamBase5GetFDERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(node_http2.o)
duplicate symbol __ZN4node10StreamBase15GetBytesWrittenERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(pipe_wrap.o)
duplicate symbol __ZN4node10StreamBase11GetExternalERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(pipe_wrap.o)
duplicate symbol __ZN4node10StreamBase12GetBytesReadERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(pipe_wrap.o)
duplicate symbol __ZN4node10StreamBase5GetFDERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(pipe_wrap.o)
duplicate symbol __ZN4node10StreamBase15GetBytesWrittenERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(process_wrap.o)
duplicate symbol __ZN4node10StreamBase11GetExternalERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(process_wrap.o)
duplicate symbol __ZN4node10StreamBase12GetBytesReadERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(process_wrap.o)
duplicate symbol __ZN4node10StreamBase5GetFDERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(process_wrap.o)
duplicate symbol __ZN4node10StreamBase15GetBytesWrittenERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(stream_base.o)
duplicate symbol __ZN4node10StreamBase11GetExternalERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(stream_base.o)
duplicate symbol __ZN4node10StreamBase12GetBytesReadERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(stream_base.o)
duplicate symbol __ZN4node10StreamBase5GetFDERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(stream_base.o)
duplicate symbol __ZN4node10StreamBase15GetBytesWrittenERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(stream_pipe.o)
duplicate symbol __ZN4node10StreamBase11GetExternalERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(stream_pipe.o)
duplicate symbol __ZN4node10StreamBase12GetBytesReadERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(stream_pipe.o)
duplicate symbol __ZN4node10StreamBase5GetFDERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(stream_pipe.o)
duplicate symbol __ZN4node10StreamBase15GetBytesWrittenERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(stream_wrap.o)
duplicate symbol __ZN4node10StreamBase11GetExternalERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(stream_wrap.o)
duplicate symbol __ZN4node10StreamBase12GetBytesReadERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(stream_wrap.o)
duplicate symbol __ZN4node10StreamBase5GetFDERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(stream_wrap.o)
duplicate symbol __ZN4node10StreamBase15GetBytesWrittenERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(tcp_wrap.o)
duplicate symbol __ZN4node10StreamBase11GetExternalERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(tcp_wrap.o)
duplicate symbol __ZN4node10StreamBase12GetBytesReadERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(tcp_wrap.o)
duplicate symbol __ZN4node10StreamBase5GetFDERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(tcp_wrap.o)
duplicate symbol __ZN4node10StreamBase15GetBytesWrittenERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(tty_wrap.o)
duplicate symbol __ZN4node10StreamBase11GetExternalERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(tty_wrap.o)
duplicate symbol __ZN4node10StreamBase12GetBytesReadERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(tty_wrap.o)
duplicate symbol __ZN4node10StreamBase5GetFDERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(tty_wrap.o)
duplicate symbol __ZN4node10StreamBase15GetBytesWrittenERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(tls_wrap.o)
duplicate symbol __ZN4node10StreamBase11GetExternalERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(tls_wrap.o)
duplicate symbol __ZN4node10StreamBase12GetBytesReadERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(tls_wrap.o)
duplicate symbol __ZN4node10StreamBase5GetFDERKN2v820FunctionCallbackInfoINS1_5ValueEEE in:
    /Users/jon/code/nodejs/node/out/Release/libnode.a(connection_wrap.o)
    /Users/jon/code/nodejs/node/out/Release/libnode.a(tls_wrap.o)
ld: 52 duplicate symbols for architecture x86_64
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 19, 2018

@maclover7 build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2010/pipeline

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Dec 19, 2018
addaleax
addaleax approved these changes Dec 19, 2018
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

… Nice!

addaleax
addaleax reviewed Dec 19, 2018
View reviewed changes
src/stream_base-inl.h Outdated
Copy link
Member

@addaleax addaleax Dec 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, we use 4 spaces of indentation here – and maybe it makes sense to turn this macro into a function?

Copy link
Member

@joyeecheung joyeecheung Dec 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tips: use CLANG_FORMAT_START=master make format-cpp to auto format, or replace master with some other commit that you are branching from

@maclover7
Copy link
Contributor Author

maclover7 commented Dec 19, 2018

updated @addaleax @joyeecheung, PTAL

addaleax
addaleax approved these changes Dec 19, 2018
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still LGTM :)

@maclover7
Copy link
Contributor Author

maclover7 commented Dec 19, 2018 •
edited
Loading

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/2013/ (✔️)
CI: https://ci.nodejs.org/job/node-test-pull-request/19687/

@maclover7
Copy link
Contributor Author

maclover7 commented Dec 21, 2018

Looks like there is an intermittent timeout issue on some platforms with test-tls-socket-close, hoping common.platformTimeout can fix...

@addaleax
Copy link
Member

addaleax commented Jan 7, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/19979/

@addaleax addaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jan 7, 2019
@addaleax
Copy link
Member

addaleax commented Jan 9, 2019

@maclover7 It looks like the issue is that some streams, in particular libuv streams, delete the associated C++ object before the JS object is garbage collected. They set the first internal field to nullptr, but they don’t know about the second internal field.

This would be a (but maybe not the best?) fix:

diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h
index 1842962192db..d672fc5a0dae 100644
--- a/src/stream_base-inl.h
+++ b/src/stream_base-inl.h
@@ -274,6 +274,9 @@ inline void StreamBase::AttachToObject(v8::Local<v8::Object> obj) {
 }
 
 inline StreamBase* StreamBase::FromObject(v8::Local<v8::Object> obj) {
+  if (obj->GetAlignedPointerFromInternalField(0) == nullptr)
+    return nullptr;
+
   return static_cast<StreamBase*>(
       obj->GetAlignedPointerFromInternalField(kStreamBaseField));
 }

@addaleax
Copy link
Member

addaleax commented Jan 13, 2019

Ping @maclover7 – barring better ideas, I can also apply the patch above and push to this PR for you?

@addaleax
Copy link
Member

addaleax commented Mar 7, 2019

Hm … ping @maclover7 again? :)

@addaleax
Copy link
Member

addaleax commented Mar 7, 2019

I’ve rebased + pushed the suggested change… @jasnell Can you confirm your approval?

CI: https://ci.nodejs.org/job/node-test-pull-request/21304/

@addaleax
Copy link
Member

addaleax commented Mar 7, 2019

Resume CI: https://ci.nodejs.org/job/node-test-pull-request/21313/

@addaleax
Copy link
Member

addaleax commented Mar 8, 2019

Landed in 4697e1b 🎉

@addaleax addaleax closed this Mar 8, 2019
addaleax pushed a commit that referenced this pull request Mar 8, 2019
@maclover7 @addaleax
src: remove templating from StreamBase
4697e1b 
PR-URL: #25142
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax added a commit to addaleax/node that referenced this pull request Mar 8, 2019
@addaleax
lib,src: remove usage of _externalStream
7888115 
Since 4697e1b, it is no longer
necessary to use `v8::External`s to pass `StreamBase` instances
to native functions.

Refs: nodejs#25142
@addaleax addaleax mentioned this pull request Mar 8, 2019
Closed
3 tasks
danbev pushed a commit that referenced this pull request Mar 11, 2019
@addaleax @danbev
lib,src: remove usage of _externalStream
560466c 
Since 4697e1b, it is no longer
necessary to use `v8::External`s to pass `StreamBase` instances
to native functions.

PR-URL: #26510
Refs: #25142
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Mar 12, 2019
@maclover7 @BridgeAR
src: remove templating from StreamBase
3b06d29 
PR-URL: nodejs#25142
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
BridgeAR pushed a commit that referenced this pull request Mar 13, 2019
@addaleax @BridgeAR
lib,src: remove usage of _externalStream
ca9bb1f 
Since 4697e1b, it is no longer
necessary to use `v8::External`s to pass `StreamBase` instances
to native functions.

PR-URL: #26510
Refs: #25142
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@BridgeAR BridgeAR mentioned this pull request Mar 13, 2019
Merged
BridgeAR pushed a commit that referenced this pull request Mar 14, 2019
@maclover7 @BridgeAR
src: remove templating from StreamBase
d688b8a 
PR-URL: #25142
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
BridgeAR pushed a commit that referenced this pull request Mar 14, 2019
@addaleax @BridgeAR
lib,src: remove usage of _externalStream
9398d84 
Since 4697e1b, it is no longer
necessary to use `v8::External`s to pass `StreamBase` instances
to native functions.

PR-URL: #26510
Refs: #25142
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@maclover7 maclover7 deleted the jm-streambase branch March 16, 2019 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joyeecheung joyeecheung joyeecheung left review comments

@jasnell jasnell jasnell approved these changes

@addaleax addaleax addaleax approved these changes

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@maclover7 @nodejs-github-bot @addaleax @jasnell @joyeecheung