From 3eb5778d1b1408d6223cdb3f5124a9b83f46f5eb Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 15 Nov 2017 15:48:49 +0100 Subject: [PATCH 01/12] http, stream: writeHWM -> writableHighWaterMark See: https://github.com/nodejs/node/pull/12860#pullrequestreview-76800871 PR-URL: https://github.com/nodejs/node/pull/17050 Reviewed-By: Calvin Metcalf Reviewed-By: James M Snell Reviewed-By: Colin Ihrig --- lib/_http_server.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/_http_server.js b/lib/_http_server.js index 736762dc26c..7315a266b89 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -373,13 +373,13 @@ function connectionListener(socket) { function updateOutgoingData(socket, state, delta) { state.outgoingData += delta; if (socket._paused && - state.outgoingData < socket.writeHWM) { + state.outgoingData < socket.writableHighWaterMark) { return socketOnDrain(socket, state); } } function socketOnDrain(socket, state) { - var needPause = state.outgoingData > socket.writeHWM; + var needPause = state.outgoingData > socket.writableHighWaterMark; // If we previously paused, then start reading again. if (socket._paused && !needPause) { From 7d8fe7df9153929aa92b55d9fb9579023be38072 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 11 Nov 2017 14:42:17 +0100 Subject: [PATCH 02/12] src: implement backtrace-on-abort for windows PR-URL: https://github.com/nodejs/node/pull/16951 Reviewed-By: Refael Ackermann Reviewed-By: James M Snell Reviewed-By: Colin Ihrig --- src/backtrace_win32.cc | 34 +++++++++++++++++++++++++++++- src/node.h | 4 ++-- test/abort/test-abort-backtrace.js | 9 ++++---- 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/backtrace_win32.cc b/src/backtrace_win32.cc index 71610fd663b..ccc3af9455b 100644 --- a/src/backtrace_win32.cc +++ b/src/backtrace_win32.cc @@ -1,8 +1,40 @@ -#include "node.h" +#include "node_internals.h" +#include +#include namespace node { void DumpBacktrace(FILE* fp) { + void* frames[256]; + int size = CaptureStackBackTrace(0, arraysize(frames), frames, nullptr); + HANDLE process = GetCurrentProcess(); + (void)SymInitialize(process, nullptr, true); + + // Ref: https://msdn.microsoft.com/en-en/library/windows/desktop/ms680578(v=vs.85).aspx + char info_buf[sizeof(SYMBOL_INFO) + MAX_SYM_NAME]; + SYMBOL_INFO* info = reinterpret_cast(info_buf); + char demangled[MAX_SYM_NAME]; + + for (int i = 1; i < size; i += 1) { + void* frame = frames[i]; + fprintf(fp, "%2d: ", i); + info->MaxNameLen = MAX_SYM_NAME; + info->SizeOfStruct = sizeof(SYMBOL_INFO); + const bool have_info = + SymFromAddr(process, reinterpret_cast(frame), nullptr, info); + if (!have_info || strlen(info->Name) == 0) { + fprintf(fp, "%p", frame); + } else if (UnDecorateSymbolName(info->Name, + demangled, + sizeof(demangled), + UNDNAME_COMPLETE)) { + fprintf(fp, "%s", demangled); + } else { + fprintf(fp, "%s", info->Name); + } + fprintf(fp, "\n"); + } + (void)SymCleanup(process); } } // namespace node diff --git a/src/node.h b/src/node.h index 3cf5692f6b2..5f74854891b 100644 --- a/src/node.h +++ b/src/node.h @@ -40,10 +40,10 @@ #endif // This should be defined in make system. -// See issue https://github.com/joyent/node/issues/1236 +// See issue https://github.com/nodejs/node-v0.x-archive/issues/1236 #if defined(__MINGW32__) || defined(_MSC_VER) #ifndef _WIN32_WINNT -# define _WIN32_WINNT 0x0501 +# define _WIN32_WINNT 0x0600 // Windows Server 2008 #endif #ifndef NOMINMAX diff --git a/test/abort/test-abort-backtrace.js b/test/abort/test-abort-backtrace.js index 6d3121e538e..e69ac3ddfde 100644 --- a/test/abort/test-abort-backtrace.js +++ b/test/abort/test-abort-backtrace.js @@ -1,8 +1,5 @@ 'use strict'; const common = require('../common'); -if (common.isWindows) - common.skip('Backtraces unimplemented on Windows.'); - const assert = require('assert'); const cp = require('child_process'); @@ -21,8 +18,10 @@ if (process.argv[2] === 'child') { assert.fail(`Each frame should start with a frame number:\n${stderr}`); } - if (!frames.some((frame) => frame.includes(`[${process.execPath}]`))) { - assert.fail(`Some frames should include the binary name:\n${stderr}`); + if (!common.isWindows) { + if (!frames.some((frame) => frame.includes(`[${process.execPath}]`))) { + assert.fail(`Some frames should include the binary name:\n${stderr}`); + } } } } From 74e7a4a041fe83f106bc50ebe99c961afb28a0cd Mon Sep 17 00:00:00 2001 From: Steve Kinney Date: Sat, 4 Nov 2017 16:08:37 -0600 Subject: [PATCH 03/12] test: add basic WebAssembly test Tests a basic WebAssembly module that adds two numbers. wasm example from the WebAssembly/wabt repo licensed Apache 2.0. Refs: https://github.com/WebAssembly/wabt/blob/49b7984544ddaf14d5e2f1ad9115dad7e9a2b299/demo/wat2wasm/examples.js#L27-L32 PR-URL: https://github.com/nodejs/node/pull/16760 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Myles Borins Reviewed-By: Sakthipriyan Vairamani Reviewed-By: Yuta Hiroto --- test/.eslintrc.yaml | 4 ++++ test/fixtures/test.wasm | Bin 0 -> 44 bytes test/parallel/test-wasm-simple.js | 18 ++++++++++++++++++ 3 files changed, 22 insertions(+) create mode 100644 test/fixtures/test.wasm create mode 100644 test/parallel/test-wasm-simple.js diff --git a/test/.eslintrc.yaml b/test/.eslintrc.yaml index 6be0ca4e3f8..aa320996aa4 100644 --- a/test/.eslintrc.yaml +++ b/test/.eslintrc.yaml @@ -15,3 +15,7 @@ rules: inspector-check: error ## common module is mandatory in tests required-modules: [error, common] + +# Global scoped methods and vars +globals: + WebAssembly: false diff --git a/test/fixtures/test.wasm b/test/fixtures/test.wasm new file mode 100644 index 0000000000000000000000000000000000000000..8b19588df228b76c6747d04aee091cbe268592d0 GIT binary patch literal 44 zcmZQbEY4+QU|?WmXG~zKuV<`hW@2Pu=VD|_Oi2kT&u3uZ;$&oJP+(AC%;E+BoDK$l literal 0 HcmV?d00001 diff --git a/test/parallel/test-wasm-simple.js b/test/parallel/test-wasm-simple.js new file mode 100644 index 00000000000..6227875fd8a --- /dev/null +++ b/test/parallel/test-wasm-simple.js @@ -0,0 +1,18 @@ +'use strict'; + +require('../common'); + +const assert = require('assert'); +const fixtures = require('../common/fixtures'); + +const buffer = fixtures.readSync('test.wasm'); + +assert.ok(WebAssembly.validate(buffer), 'Buffer should be valid WebAssembly'); + +WebAssembly.instantiate(buffer, {}).then((results) => { + assert.strictEqual( + results.instance.exports.addTwo(10, 20), + 30, + 'Exported function should add two numbers.', + ); +}); From 1b093cb93df05bea0c9adfb6a060d2a4542c1ae0 Mon Sep 17 00:00:00 2001 From: Franziska Hinkelmann Date: Mon, 13 Nov 2017 22:50:33 +0100 Subject: [PATCH 04/12] src: use unique_ptr for requests in crypto Instead of raw pointerns, use std::unique_ptr for PBKDF2Request and RandomBytesRequest. This makes ownership more clear. PR-URL: https://github.com/nodejs/node/pull/17000 Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis Reviewed-By: Colin Ihrig --- src/node_crypto.cc | 48 ++++++++++++++++++---------------------------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 71fe303fd69..cfab04b11ea 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -5534,9 +5534,9 @@ void PBKDF2Request::After() { void PBKDF2Request::After(uv_work_t* work_req, int status) { CHECK_EQ(status, 0); - PBKDF2Request* req = ContainerOf(&PBKDF2Request::work_req_, work_req); + std::unique_ptr req( + ContainerOf(&PBKDF2Request::work_req_, work_req)); req->After(); - delete req; } @@ -5551,7 +5551,6 @@ void PBKDF2(const FunctionCallbackInfo& args) { double raw_keylen = -1; int keylen = -1; int iter = -1; - PBKDF2Request* req = nullptr; Local obj; passlen = Buffer::Length(args[0]); @@ -5587,15 +5586,9 @@ void PBKDF2(const FunctionCallbackInfo& args) { obj = env->pbkdf2_constructor_template()-> NewInstance(env->context()).ToLocalChecked(); - req = new PBKDF2Request(env, - obj, - digest, - passlen, - pass, - saltlen, - salt, - iter, - keylen); + std::unique_ptr req( + new PBKDF2Request(env, obj, digest, passlen, pass, saltlen, salt, iter, + keylen)); if (args[5]->IsFunction()) { obj->Set(env->ondone_string(), args[5]); @@ -5608,7 +5601,7 @@ void PBKDF2(const FunctionCallbackInfo& args) { } uv_queue_work(env->event_loop(), - req->work_req(), + req.release()->work_req(), PBKDF2Request::Work, PBKDF2Request::After); } else { @@ -5616,7 +5609,6 @@ void PBKDF2(const FunctionCallbackInfo& args) { req->Work(); Local argv[2]; req->After(&argv); - delete req; if (argv[0]->IsObject()) env->isolate()->ThrowException(argv[0]); @@ -5754,25 +5746,23 @@ void RandomBytesCheck(RandomBytesRequest* req, Local (*argv)[2]) { void RandomBytesAfter(uv_work_t* work_req, int status) { CHECK_EQ(status, 0); - RandomBytesRequest* req = - ContainerOf(&RandomBytesRequest::work_req_, work_req); + std::unique_ptr req( + ContainerOf(&RandomBytesRequest::work_req_, work_req)); Environment* env = req->env(); HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); Local argv[2]; - RandomBytesCheck(req, &argv); + RandomBytesCheck(req.get(), &argv); req->MakeCallback(env->ondone_string(), arraysize(argv), argv); - delete req; } void RandomBytesProcessSync(Environment* env, - RandomBytesRequest* req, + std::unique_ptr req, Local (*argv)[2]) { env->PrintSyncTrace(); RandomBytesWork(req->work_req()); - RandomBytesCheck(req, argv); - delete req; + RandomBytesCheck(req.get(), argv); if (!(*argv)[0]->IsNull()) env->isolate()->ThrowException((*argv)[0]); @@ -5788,12 +5778,12 @@ void RandomBytes(const FunctionCallbackInfo& args) { Local obj = env->randombytes_constructor_template()-> NewInstance(env->context()).ToLocalChecked(); char* data = node::Malloc(size); - RandomBytesRequest* req = + std::unique_ptr req( new RandomBytesRequest(env, obj, size, data, - RandomBytesRequest::FREE_DATA); + RandomBytesRequest::FREE_DATA)); if (args[1]->IsFunction()) { obj->Set(env->ondone_string(), args[1]); @@ -5806,13 +5796,13 @@ void RandomBytes(const FunctionCallbackInfo& args) { } uv_queue_work(env->event_loop(), - req->work_req(), + req.release()->work_req(), RandomBytesWork, RandomBytesAfter); args.GetReturnValue().Set(obj); } else { Local argv[2]; - RandomBytesProcessSync(env, req, &argv); + RandomBytesProcessSync(env, std::move(req), &argv); if (argv[0]->IsNull()) args.GetReturnValue().Set(argv[1]); } @@ -5835,12 +5825,12 @@ void RandomBytesBuffer(const FunctionCallbackInfo& args) { char* data = Buffer::Data(args[0]); data += offset; - RandomBytesRequest* req = + std::unique_ptr req( new RandomBytesRequest(env, obj, size, data, - RandomBytesRequest::DONT_FREE_DATA); + RandomBytesRequest::DONT_FREE_DATA)); if (args[3]->IsFunction()) { obj->Set(env->context(), env->ondone_string(), args[3]).FromJust(); @@ -5852,13 +5842,13 @@ void RandomBytesBuffer(const FunctionCallbackInfo& args) { } uv_queue_work(env->event_loop(), - req->work_req(), + req.release()->work_req(), RandomBytesWork, RandomBytesAfter); args.GetReturnValue().Set(obj); } else { Local argv[2]; - RandomBytesProcessSync(env, req, &argv); + RandomBytesProcessSync(env, std::move(req), &argv); if (argv[0]->IsNull()) args.GetReturnValue().Set(argv[1]); } From a0b1e2eca684a21e87dd8ae1d5becb4c85e4ddd4 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Tue, 14 Nov 2017 08:41:10 +0100 Subject: [PATCH 05/12] build: prevent echoing of recipes for test target Currenlty the test target will echo additional information that might not be that useful, for example: make doc-only make[1]: Nothing to be done for `doc-only'. make lint Running JS linter... Running C++ linter... Total errors found: 0 make[2]: Nothing to be done for `lint-md'. Running C++ linter on addon docs... Total errors found: 0 make cctest This commit suggests reducing this to: make -s doc-only make -s lint Running JS linter... Running C++ linter... Running C++ linter on addon docs... Total errors found: 0 make -s cctest PR-URL: https://github.com/nodejs/node/pull/17010 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Joyee Cheung --- Makefile | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index b2facee41a8..4ab99b5bd31 100644 --- a/Makefile +++ b/Makefile @@ -210,11 +210,11 @@ test: all $(MAKE) cctest else test: all - $(MAKE) build-addons - $(MAKE) build-addons-napi - $(MAKE) doc-only - $(MAKE) lint - $(MAKE) cctest + $(MAKE) -s build-addons + $(MAKE) -s build-addons-napi + $(MAKE) -s doc-only + $(MAKE) -s lint + $(MAKE) -s cctest $(PYTHON) tools/test.py --mode=release -J \ $(CI_ASYNC_HOOKS) \ $(CI_JS_SUITES) \ From b021403b2afdab0f5077b55aa27286c187119acd Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Wed, 23 Aug 2017 09:52:59 +0200 Subject: [PATCH 06/12] test: --enable-static linked executable The motivation for this commit is to enable projects embedding Node.js and building with --enable-static to be able to run the test suite and linter. Currently when building with --enable-static no node executable will be created which means that the tests (apart from the cctest) and linter cannot be run. This is currently a work in progress and works on MacOS but I need to run the CI, and manually on different environments to verify that it works as expected. PR-URL: https://github.com/nodejs/node/pull/14986 Refs: https://github.com/nodejs/node/issues/14158 Refs: https://github.com/nodejs/node/pull/14892 Reviewed-By: James M Snell Reviewed-By: Refael Ackermann Reviewed-By: Ben Noordhuis Reviewed-By: Gireesh Punathil --- common.gypi | 4 +- node.gyp | 61 ++++++++++++++++++++++--- node.gypi | 4 +- test/addons/openssl-binding/binding.gyp | 19 ++++++-- test/addons/zlib-binding/binding.gyp | 13 ++++++ 5 files changed, 87 insertions(+), 14 deletions(-) diff --git a/common.gypi b/common.gypi index 302d494e9e3..5cc75d763f9 100644 --- a/common.gypi +++ b/common.gypi @@ -64,9 +64,9 @@ 'V8_BASE': '<(PRODUCT_DIR)/libv8_base.a', }], ['openssl_fips != ""', { - 'OPENSSL_PRODUCT': 'libcrypto.a', + 'OPENSSL_PRODUCT': '<(STATIC_LIB_PREFIX)crypto<(STATIC_LIB_SUFFIX)', }, { - 'OPENSSL_PRODUCT': 'libopenssl.a', + 'OPENSSL_PRODUCT': '<(STATIC_LIB_PREFIX)openssl<(STATIC_LIB_SUFFIX)', }], ['OS=="mac"', { 'clang%': 1, diff --git a/node.gyp b/node.gyp index 63ace809396..d0fa50c8696 100644 --- a/node.gyp +++ b/node.gyp @@ -341,7 +341,13 @@ [ 'OS=="win"', { 'sources': [ 'src/backtrace_win32.cc', - 'src/res/node.rc', + ], + 'conditions': [ + [ 'node_target_type!="static_library"', { + 'sources': [ + 'src/res/node.rc', + ], + }], ], 'defines!': [ 'NODE_PLATFORM="win"', @@ -508,7 +514,7 @@ 'target_name': 'node_etw', 'type': 'none', 'conditions': [ - [ 'node_use_etw=="true"', { + [ 'node_use_etw=="true" and node_target_type!="static_library"', { 'actions': [ { 'action_name': 'node_etw', @@ -529,7 +535,7 @@ 'target_name': 'node_perfctr', 'type': 'none', 'conditions': [ - [ 'node_use_perfctr=="true"', { + [ 'node_use_perfctr=="true" and node_target_type!="static_library"', { 'actions': [ { 'action_name': 'node_perfctr_man', @@ -591,13 +597,15 @@ '<(SHARED_INTERMEDIATE_DIR)/node_javascript.cc', ], 'conditions': [ - [ 'node_use_dtrace=="false" and node_use_etw=="false"', { + [ 'node_use_dtrace=="false" and node_use_etw=="false" or ' + 'node_target_type=="static_library"', { 'inputs': [ 'src/notrace_macros.py' ] }], - ['node_use_lttng=="false"', { + ['node_use_lttng=="false" or node_target_type=="static_library"', { 'inputs': [ 'src/nolttng_macros.py' ] }], - [ 'node_use_perfctr=="false"', { + [ 'node_use_perfctr=="false" or ' + 'node_target_type=="static_library"', { 'inputs': [ 'src/noperfctr_macros.py' ] }] ], @@ -952,6 +960,47 @@ ], # end targets 'conditions': [ + [ 'node_target_type=="static_library"', { + 'targets': [ + { + 'target_name': 'static_node', + 'type': 'executable', + 'product_name': '<(node_core_target_name)', + 'dependencies': [ + '<(node_core_target_name)', + ], + 'sources+': [ + 'src/node_main.cc', + ], + 'include_dirs': [ + 'deps/v8/include', + ], + 'xcode_settings': { + 'OTHER_LDFLAGS': [ + '-Wl,-force_load,<(PRODUCT_DIR)/<(STATIC_LIB_PREFIX)' + '<(node_core_target_name)<(STATIC_LIB_SUFFIX)', + ], + }, + 'msvs_settings': { + 'VCLinkerTool': { + 'AdditionalOptions': [ + '/WHOLEARCHIVE:<(PRODUCT_DIR)/lib/' + '<(node_core_target_name)<(STATIC_LIB_SUFFIX)', + ], + }, + }, + 'conditions': [ + ['OS in "linux freebsd openbsd solaris android"', { + 'ldflags': [ + '-Wl,--whole-archive,<(OBJ_DIR)/<(STATIC_LIB_PREFIX)' + '<(node_core_target_name)<(STATIC_LIB_SUFFIX)', + '-Wl,--no-whole-archive', + ], + }], + ], + }, + ], + }], ['OS=="aix"', { 'targets': [ { diff --git a/node.gypi b/node.gypi index 718ef0c9e7f..ec78df2a339 100644 --- a/node.gypi +++ b/node.gypi @@ -78,7 +78,7 @@ 'src/node_lttng.cc' ], } ], - [ 'node_use_etw=="true"', { + [ 'node_use_etw=="true" and node_target_type!="static_library"', { 'defines': [ 'HAVE_ETW=1' ], 'dependencies': [ 'node_etw' ], 'sources': [ @@ -90,7 +90,7 @@ 'tools/msvs/genfiles/node_etw_provider.rc', ] } ], - [ 'node_use_perfctr=="true"', { + [ 'node_use_perfctr=="true" and node_target_type!="static_library"', { 'defines': [ 'HAVE_PERFCTR=1' ], 'dependencies': [ 'node_perfctr' ], 'sources': [ diff --git a/test/addons/openssl-binding/binding.gyp b/test/addons/openssl-binding/binding.gyp index bafde41348c..425b38caa3f 100644 --- a/test/addons/openssl-binding/binding.gyp +++ b/test/addons/openssl-binding/binding.gyp @@ -1,12 +1,23 @@ { + 'includes': ['../../../config.gypi'], + 'variables': { + 'node_target_type%': '', + }, 'targets': [ { 'target_name': 'binding', 'conditions': [ - ['node_use_openssl=="true"', { - 'sources': ['binding.cc'], - 'include_dirs': ['../../../deps/openssl/openssl/include'], - }] + ['node_use_openssl=="true"', { + 'sources': ['binding.cc'], + 'include_dirs': ['../../../deps/openssl/openssl/include'], + 'conditions': [ + ['OS=="win" and node_target_type=="static_library"', { + 'libraries': [ + '../../../../$(Configuration)/lib/<(OPENSSL_PRODUCT)' + ], + }], + ], + }] ] }, ] diff --git a/test/addons/zlib-binding/binding.gyp b/test/addons/zlib-binding/binding.gyp index 60a9bb82661..24c3ae78a24 100644 --- a/test/addons/zlib-binding/binding.gyp +++ b/test/addons/zlib-binding/binding.gyp @@ -1,9 +1,22 @@ { + 'includes': ['../../../config.gypi'], + 'variables': { + 'node_target_type%': '', + }, 'targets': [ { 'target_name': 'binding', 'sources': ['binding.cc'], 'include_dirs': ['../../../deps/zlib'], + 'conditions': [ + ['node_target_type=="static_library"', { + 'conditions': [ + ['OS=="win"', { + 'libraries': ['../../../../$(Configuration)/lib/zlib.lib'], + }], + ], + }], + ], }, ] } From 9ae81b9cb923520baa2d4e7a15ad21d2326f9bb8 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Mon, 9 Oct 2017 14:50:24 +0200 Subject: [PATCH 07/12] Revert "build: for --enable-static, run only cctest" This reverts commit a36b5405029597ce09e15373a321c47930689c08. PR-URL: https://github.com/nodejs/node/pull/14986 Refs: https://github.com/nodejs/node/issues/14158 Refs: https://github.com/nodejs/node/pull/14892 Reviewed-By: James M Snell Reviewed-By: Refael Ackermann Reviewed-By: Ben Noordhuis Reviewed-By: Gireesh Punathil --- Makefile | 5 ----- configure | 2 -- vcbuild.bat | 6 ++---- 3 files changed, 2 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 4ab99b5bd31..e29b486cec5 100644 --- a/Makefile +++ b/Makefile @@ -205,10 +205,6 @@ v8: tools/make-v8.sh $(MAKE) -C deps/v8 $(V8_ARCH).$(BUILDTYPE_LOWER) $(V8_BUILD_OPTIONS) -ifeq ($(NODE_TARGET_TYPE),static_library) -test: all - $(MAKE) cctest -else test: all $(MAKE) -s build-addons $(MAKE) -s build-addons-napi @@ -221,7 +217,6 @@ test: all $(CI_NATIVE_SUITES) \ $(CI_DOC) \ known_issues -endif # For a quick test, does not run linter or build doc test-only: all diff --git a/configure b/configure index 0c5aed8fcc0..bbccf05fb47 100755 --- a/configure +++ b/configure @@ -1461,8 +1461,6 @@ config = { 'BUILDTYPE': 'Debug' if options.debug else 'Release', 'USE_XCODE': str(int(options.use_xcode or 0)), 'PYTHON': sys.executable, - 'NODE_TARGET_TYPE': variables['node_target_type'] if options.enable_static \ - else '', } if options.prefix: diff --git a/vcbuild.bat b/vcbuild.bat index d2683f99887..a765bcb439b 100644 --- a/vcbuild.bat +++ b/vcbuild.bat @@ -110,7 +110,7 @@ if /i "%1"=="download-all" set download_arg="--download=all"&goto arg-ok if /i "%1"=="ignore-flaky" set test_args=%test_args% --flaky-tests=dontcare&goto arg-ok if /i "%1"=="enable-vtune" set enable_vtune_arg=1&goto arg-ok if /i "%1"=="dll" set dll=1&goto arg-ok -if /i "%1"=="static" set enable_static=1&goto arg-ok +if /i "%1"=="static" set enable_static=1&goto arg-ok if /i "%1"=="no-NODE-OPTIONS" set no_NODE_OPTIONS=1&goto arg-ok if /i "%1"=="debug-http2" set debug_http2=1&goto arg-ok if /i "%1"=="debug-nghttp2" set debug_nghttp2=1&goto arg-ok @@ -465,9 +465,8 @@ if "%config%"=="Debug" set test_args=--mode=debug %test_args% if "%config%"=="Release" set test_args=--mode=release %test_args% echo running 'cctest %cctest_args%' "%config%\cctest" %cctest_args% -REM when building a static library there's no binary to run tests -if defined enable_static goto test-v8 call :run-python tools\test.py %test_args% +goto test-v8 :test-v8 if not defined custom_v8_test goto lint-cpp @@ -520,7 +519,6 @@ set "localcppfilelist=%localcppfilelist% %1" goto exit :lint-js -if defined enable_static goto exit if defined lint_js_ci goto lint-js-ci if not defined lint_js goto exit if not exist tools\eslint goto no-lint From 787863ddb4296d2feb84fd5ca7b49a544edf727c Mon Sep 17 00:00:00 2001 From: Franziska Hinkelmann Date: Sun, 12 Nov 2017 22:16:10 +0100 Subject: [PATCH 08/12] src: use unique pointer for tracing_agent Use std::unique_ptr instead of raw pointers for the tracing_agent_ in node.cc. This makes ownership clearer and we don't risk a memory leak. PR-URL: https://github.com/nodejs/node/pull/17012 Reviewed-By: Daniel Bevenius Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Timothy Gu --- src/node.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/node.cc b/src/node.cc index f688d41fc35..eeab7df11fb 100644 --- a/src/node.cc +++ b/src/node.cc @@ -273,8 +273,8 @@ node::DebugOptions debug_options; static struct { #if NODE_USE_V8_PLATFORM void Initialize(int thread_pool_size) { - tracing_agent_ = - trace_enabled ? new tracing::Agent() : nullptr; + tracing_agent_.reset( + trace_enabled ? new tracing::Agent() : nullptr); platform_ = new NodePlatform(thread_pool_size, trace_enabled ? tracing_agent_->GetTracingController() : nullptr); V8::InitializePlatform(platform_); @@ -286,8 +286,7 @@ static struct { platform_->Shutdown(); delete platform_; platform_ = nullptr; - delete tracing_agent_; - tracing_agent_ = nullptr; + tracing_agent_.reset(nullptr); } void DrainVMTasks(Isolate* isolate) { @@ -324,7 +323,7 @@ static struct { return platform_; } - tracing::Agent* tracing_agent_; + std::unique_ptr tracing_agent_; NodePlatform* platform_; #else // !NODE_USE_V8_PLATFORM void Initialize(int thread_pool_size) {} From ed0327b8868cc3df981f81be6409586b97d06ac8 Mon Sep 17 00:00:00 2001 From: Franziska Hinkelmann Date: Wed, 15 Nov 2017 18:05:23 +0100 Subject: [PATCH 09/12] doc: add Table of Contents to Cpp style guide The Cpp style guide is growing. IMHO, a Table of Contents makes it easier to navigate. PR-URL: https://github.com/nodejs/node/pull/17052 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Richard Lau Reviewed-By: Daniel Bevenius --- CPP_STYLE_GUIDE.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CPP_STYLE_GUIDE.md b/CPP_STYLE_GUIDE.md index 3bea5bb1075..3c552fc2bf2 100644 --- a/CPP_STYLE_GUIDE.md +++ b/CPP_STYLE_GUIDE.md @@ -1,5 +1,22 @@ # C++ Style Guide +## Table of Contents + +* [Left-leaning (C++ style) asterisks for pointer declarations](#left-leaning-c-style-asterisks-for-pointer-declarations) +* [2 spaces of indentation for blocks or bodies of conditionals](#2-spaces-of-indentation-for-blocks-or-bodies-of-conditionals) +* [4 spaces of indentation for statement continuations](#4-spaces-of-indentation-for-statement-continuations) +* [Align function arguments vertically](#align-function-arguments-vertically) +* [Initialization lists](#initialization-lists) +* [CamelCase for methods, functions and classes](#camelcase-for-methods-functions-and-classes) +* [snake\_case for local variables and parameters](#snake_case-for-local-variables-and-parameters) +* [snake\_case\_ for private class fields](#snake_case_-for-private-class-fields) +* [Space after `template`](#space-after-template) +* [Type casting](#type-casting) +* [Memory allocation](#memory-allocation) +* [`nullptr` instead of `NULL` or `0`](#nullptr-instead-of-null-or-0) +* [Do not include `*.h` if `*-inl.h` has already been included](#do-not-include-h-if--inlh-has-already-been-included) +* [Avoid throwing JavaScript errors in nested C++ methods](#avoid-throwing-javascript-errors-in-nested-c-methods) + Unfortunately, the C++ linter (based on [Google’s `cpplint`](https://github.com/google/styleguide)), which can be run explicitly via `make lint-cpp`, does not currently catch a lot of rules that are From d217b2850efb9005819d55b697a37cbe5bd0003c Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Mon, 17 Jul 2017 16:47:12 -0700 Subject: [PATCH 10/12] async_hooks: add trace events to async_hooks This will allow trace event to record timing information for all asynchronous operations that are observed by async_hooks. PR-URL: https://github.com/nodejs/node/pull/15538 Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- doc/api/tracing.md | 4 +- lib/_http_common.js | 3 +- lib/internal/bootstrap_node.js | 1 + lib/internal/trace_events_async_hooks.js | 67 +++++++++ node.gyp | 2 + src/async-wrap.cc | 102 ++++++++++--- src/async-wrap.h | 14 +- src/node.cc | 21 ++- src/node_http_parser.cc | 13 ++ src/node_trace_events.cc | 136 ++++++++++++++++++ src/tracing/agent.cc | 1 + test/parallel/test-trace-event.js | 41 ------ test/parallel/test-trace-events-all.js | 56 ++++++++ .../parallel/test-trace-events-async-hooks.js | 58 ++++++++ test/parallel/test-trace-events-binding.js | 48 +++++++ .../test-trace-events-category-used.js | 35 +++++ test/parallel/test-trace-events-none.js | 20 +++ test/parallel/test-trace-events-v8.js | 58 ++++++++ 18 files changed, 607 insertions(+), 73 deletions(-) create mode 100644 lib/internal/trace_events_async_hooks.js create mode 100644 src/node_trace_events.cc delete mode 100644 test/parallel/test-trace-event.js create mode 100644 test/parallel/test-trace-events-all.js create mode 100644 test/parallel/test-trace-events-async-hooks.js create mode 100644 test/parallel/test-trace-events-binding.js create mode 100644 test/parallel/test-trace-events-category-used.js create mode 100644 test/parallel/test-trace-events-none.js create mode 100644 test/parallel/test-trace-events-v8.js diff --git a/doc/api/tracing.md b/doc/api/tracing.md index 28e488201ec..53c6dac55da 100644 --- a/doc/api/tracing.md +++ b/doc/api/tracing.md @@ -8,10 +8,10 @@ Node.js application. The set of categories for which traces are recorded can be specified using the `--trace-event-categories` flag followed by a list of comma separated category names. -By default the `node` and `v8` categories are enabled. +By default the `node`, `node.async_hooks`, and `v8` categories are enabled. ```txt -node --trace-events-enabled --trace-event-categories v8,node server.js +node --trace-events-enabled --trace-event-categories v8,node,node.async_hooks server.js ``` Running Node.js with tracing enabled will produce log files that can be opened diff --git a/lib/_http_common.js b/lib/_http_common.js index 4c8d3c3850b..6f5378fbe27 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -27,7 +27,6 @@ const { methods, HTTPParser } = binding; const FreeList = require('internal/freelist'); const { ondrain } = require('internal/http'); const incoming = require('_http_incoming'); -const { emitDestroy } = require('async_hooks'); const { IncomingMessage, readStart, @@ -218,7 +217,7 @@ function freeParser(parser, req, socket) { } else { // Since the Parser destructor isn't going to run the destroy() callbacks // it needs to be triggered manually. - emitDestroy(parser.getAsyncId()); + parser.free(); } } if (req) { diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index d098eab70b2..b86caf61a5c 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -59,6 +59,7 @@ if (global.__coverage__) NativeModule.require('internal/process/write-coverage').setup(); + NativeModule.require('internal/trace_events_async_hooks').setup(); NativeModule.require('internal/inspector_async_hook').setup(); // Do not initialize channel in debugger agent, it deletes env variable diff --git a/lib/internal/trace_events_async_hooks.js b/lib/internal/trace_events_async_hooks.js new file mode 100644 index 00000000000..9f47f2aa5fd --- /dev/null +++ b/lib/internal/trace_events_async_hooks.js @@ -0,0 +1,67 @@ +'use strict'; + +const trace_events = process.binding('trace_events'); +const async_wrap = process.binding('async_wrap'); +const async_hooks = require('async_hooks'); + +// Use small letters such that chrome://traceing groups by the name. +// The behaviour is not only useful but the same as the events emitted using +// the specific C++ macros. +const BEFORE_EVENT = 'b'.charCodeAt(0); +const END_EVENT = 'e'.charCodeAt(0); + +// In trace_events it is not only the id but also the name that needs to be +// repeated. Since async_hooks doesn't expose the provider type in the +// non-init events, use a map to manually map the asyncId to the type name. +const typeMemory = new Map(); + +// It is faster to emit trace_events directly from C++. Thus, this happens in +// async_wrap.cc. However, events emitted from the JavaScript API or the +// Embedder C++ API can't be emitted from async_wrap.cc. Thus they are +// emitted using the JavaScript API. To prevent emitting the same event +// twice the async_wrap.Providers list is used to filter the events. +const nativeProviders = new Set(Object.keys(async_wrap.Providers)); + +const hook = async_hooks.createHook({ + init(asyncId, type, triggerAsyncId, resource) { + if (nativeProviders.has(type)) return; + + typeMemory.set(asyncId, type); + trace_events.emit(BEFORE_EVENT, 'node.async_hooks', + type, asyncId, 'triggerAsyncId', triggerAsyncId); + }, + + before(asyncId) { + const type = typeMemory.get(asyncId); + if (type === undefined) return; + + trace_events.emit(BEFORE_EVENT, 'node.async_hooks', + type + '_CALLBACK', asyncId); + }, + + after(asyncId) { + const type = typeMemory.get(asyncId); + if (type === undefined) return; + + trace_events.emit(END_EVENT, 'node.async_hooks', + type + '_CALLBACK', asyncId); + }, + + destroy(asyncId) { + const type = typeMemory.get(asyncId); + if (type === undefined) return; + + trace_events.emit(END_EVENT, 'node.async_hooks', + type, asyncId); + + // cleanup asyncId to type map + typeMemory.delete(asyncId); + } +}); + + +exports.setup = function() { + if (trace_events.categoryGroupEnabled('node.async_hooks')) { + hook.enable(); + } +}; diff --git a/node.gyp b/node.gyp index d0fa50c8696..5bc13cec5c1 100644 --- a/node.gyp +++ b/node.gyp @@ -121,6 +121,7 @@ 'lib/internal/socket_list.js', 'lib/internal/test/unicode.js', 'lib/internal/tls.js', + 'lib/internal/trace_events_async_hooks.js', 'lib/internal/url.js', 'lib/internal/util.js', 'lib/internal/util/comparisons.js', @@ -213,6 +214,7 @@ 'src/node_platform.cc', 'src/node_perf.cc', 'src/node_serdes.cc', + 'src/node_trace_events.cc', 'src/node_url.cc', 'src/node_util.cc', 'src/node_v8.cc', diff --git a/src/async-wrap.cc b/src/async-wrap.cc index af0336d4924..c53929d11bb 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -167,18 +167,6 @@ static void DestroyAsyncIdsCallback(uv_timer_t* handle) { } -static void PushBackDestroyAsyncId(Environment* env, double id) { - if (env->async_hooks()->fields()[AsyncHooks::kDestroy] == 0) - return; - - if (env->destroy_async_id_list()->empty()) - uv_timer_start(env->destroy_async_ids_timer_handle(), - DestroyAsyncIdsCallback, 0, 0); - - env->destroy_async_id_list()->push_back(id); -} - - void AsyncWrap::EmitPromiseResolve(Environment* env, double async_id) { AsyncHooks* async_hooks = env->async_hooks(); @@ -198,6 +186,21 @@ void AsyncWrap::EmitPromiseResolve(Environment* env, double async_id) { } +void AsyncWrap::EmitTraceEventBefore() { + switch (provider_type()) { +#define V(PROVIDER) \ + case PROVIDER_ ## PROVIDER: \ + TRACE_EVENT_NESTABLE_ASYNC_BEGIN0("node.async_hooks", \ + #PROVIDER "_CALLBACK", static_cast(get_async_id())); \ + break; + NODE_ASYNC_PROVIDER_TYPES(V) +#undef V + default: + UNREACHABLE(); + } +} + + void AsyncWrap::EmitBefore(Environment* env, double async_id) { AsyncHooks* async_hooks = env->async_hooks(); @@ -217,6 +220,21 @@ void AsyncWrap::EmitBefore(Environment* env, double async_id) { } +void AsyncWrap::EmitTraceEventAfter() { + switch (provider_type()) { +#define V(PROVIDER) \ + case PROVIDER_ ## PROVIDER: \ + TRACE_EVENT_NESTABLE_ASYNC_END0("node.async_hooks", \ + #PROVIDER "_CALLBACK", static_cast(get_async_id())); \ + break; + NODE_ASYNC_PROVIDER_TYPES(V) +#undef V + default: + UNREACHABLE(); + } +} + + void AsyncWrap::EmitAfter(Environment* env, double async_id) { AsyncHooks* async_hooks = env->async_hooks(); @@ -327,8 +345,10 @@ static void PromiseHook(PromiseHookType type, Local promise, if (type == PromiseHookType::kBefore) { env->async_hooks()->push_async_ids( wrap->get_async_id(), wrap->get_trigger_async_id()); + wrap->EmitTraceEventBefore(); AsyncWrap::EmitBefore(wrap->env(), wrap->get_async_id()); } else if (type == PromiseHookType::kAfter) { + wrap->EmitTraceEventAfter(); AsyncWrap::EmitAfter(wrap->env(), wrap->get_async_id()); if (env->execution_async_id() == wrap->get_async_id()) { // This condition might not be true if async_hooks was enabled during @@ -455,7 +475,8 @@ void AsyncWrap::AsyncReset(const FunctionCallbackInfo& args) { void AsyncWrap::QueueDestroyAsyncId(const FunctionCallbackInfo& args) { CHECK(args[0]->IsNumber()); - PushBackDestroyAsyncId(Environment::GetCurrent(args), args[0]->NumberValue()); + AsyncWrap::EmitDestroy( + Environment::GetCurrent(args), args[0]->NumberValue()); } void AsyncWrap::AddWrapMethods(Environment* env, @@ -604,7 +625,34 @@ AsyncWrap::AsyncWrap(Environment* env, AsyncWrap::~AsyncWrap() { - PushBackDestroyAsyncId(env(), get_async_id()); + EmitTraceEventDestroy(); + EmitDestroy(env(), get_async_id()); +} + +void AsyncWrap::EmitTraceEventDestroy() { + switch (provider_type()) { + #define V(PROVIDER) \ + case PROVIDER_ ## PROVIDER: \ + TRACE_EVENT_NESTABLE_ASYNC_END0("node.async_hooks", \ + #PROVIDER, static_cast(get_async_id())); \ + break; + NODE_ASYNC_PROVIDER_TYPES(V) + #undef V + default: + UNREACHABLE(); + } +} + +void AsyncWrap::EmitDestroy(Environment* env, double async_id) { + if (env->async_hooks()->fields()[AsyncHooks::kDestroy] == 0) + return; + + if (env->destroy_async_id_list()->empty()) { + uv_timer_start(env->destroy_async_ids_timer_handle(), + DestroyAsyncIdsCallback, 0, 0); + } + + env->destroy_async_id_list()->push_back(async_id); } @@ -616,6 +664,19 @@ void AsyncWrap::AsyncReset(double execution_async_id, bool silent) { execution_async_id == -1 ? env()->new_async_id() : execution_async_id; trigger_async_id_ = env()->get_init_trigger_async_id(); + switch (provider_type()) { +#define V(PROVIDER) \ + case PROVIDER_ ## PROVIDER: \ + TRACE_EVENT_NESTABLE_ASYNC_BEGIN1("node.async_hooks", \ + #PROVIDER, static_cast(get_async_id()), \ + "triggerAsyncId", static_cast(get_trigger_async_id())); \ + break; + NODE_ASYNC_PROVIDER_TYPES(V) +#undef V + default: + UNREACHABLE(); + } + if (silent) return; EmitAsyncInit(env(), object(), @@ -662,8 +723,15 @@ void AsyncWrap::EmitAsyncInit(Environment* env, MaybeLocal AsyncWrap::MakeCallback(const Local cb, int argc, Local* argv) { + EmitTraceEventBefore(); + async_context context { get_async_id(), get_trigger_async_id() }; - return InternalMakeCallback(env(), object(), cb, argc, argv, context); + MaybeLocal ret = InternalMakeCallback( + env(), object(), cb, argc, argv, context); + + EmitTraceEventAfter(); + + return ret; } @@ -713,8 +781,8 @@ async_context EmitAsyncInit(Isolate* isolate, } void EmitAsyncDestroy(Isolate* isolate, async_context asyncContext) { - PushBackDestroyAsyncId(Environment::GetCurrent(isolate), - asyncContext.async_id); + AsyncWrap::EmitDestroy( + Environment::GetCurrent(isolate), asyncContext.async_id); } } // namespace node diff --git a/src/async-wrap.h b/src/async-wrap.h index 2702cfa2b77..39b6f287c93 100644 --- a/src/async-wrap.h +++ b/src/async-wrap.h @@ -128,12 +128,18 @@ class AsyncWrap : public BaseObject { static void EmitAsyncInit(Environment* env, v8::Local object, v8::Local type, - double id, + double async_id, double trigger_async_id); - static void EmitBefore(Environment* env, double id); - static void EmitAfter(Environment* env, double id); - static void EmitPromiseResolve(Environment* env, double id); + static void EmitDestroy(Environment* env, double async_id); + static void EmitBefore(Environment* env, double async_id); + static void EmitAfter(Environment* env, double async_id); + static void EmitPromiseResolve(Environment* env, double async_id); + + void EmitTraceEventBefore(); + void EmitTraceEventAfter(); + void EmitTraceEventDestroy(); + inline ProviderType provider_type() const; diff --git a/src/node.cc b/src/node.cc index eeab7df11fb..85fc23bbd93 100644 --- a/src/node.cc +++ b/src/node.cc @@ -273,13 +273,20 @@ node::DebugOptions debug_options; static struct { #if NODE_USE_V8_PLATFORM void Initialize(int thread_pool_size) { - tracing_agent_.reset( - trace_enabled ? new tracing::Agent() : nullptr); - platform_ = new NodePlatform(thread_pool_size, - trace_enabled ? tracing_agent_->GetTracingController() : nullptr); - V8::InitializePlatform(platform_); - tracing::TraceEventHelper::SetTracingController( - trace_enabled ? tracing_agent_->GetTracingController() : nullptr); + if (trace_enabled) { + tracing_agent_.reset(new tracing::Agent()); + platform_ = new NodePlatform(thread_pool_size, + tracing_agent_->GetTracingController()); + V8::InitializePlatform(platform_); + tracing::TraceEventHelper::SetTracingController( + tracing_agent_->GetTracingController()); + } else { + tracing_agent_.reset(nullptr); + platform_ = new NodePlatform(thread_pool_size, nullptr); + V8::InitializePlatform(platform_); + tracing::TraceEventHelper::SetTracingController( + new v8::TracingController()); + } } void Dispose() { diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index ccb09426096..9974f566a8c 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -397,6 +397,18 @@ class Parser : public AsyncWrap { } + static void Free(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Parser* parser; + ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder()); + + // Since the Parser destructor isn't going to run the destroy() callbacks + // it needs to be triggered manually. + parser->EmitTraceEventDestroy(); + parser->EmitDestroy(env, parser->get_async_id()); + } + + void Save() { url_.Save(); status_message_.Save(); @@ -792,6 +804,7 @@ void InitHttpParser(Local target, AsyncWrap::AddWrapMethods(env, t); env->SetProtoMethod(t, "close", Parser::Close); + env->SetProtoMethod(t, "free", Parser::Free); env->SetProtoMethod(t, "execute", Parser::Execute); env->SetProtoMethod(t, "finish", Parser::Finish); env->SetProtoMethod(t, "reinitialize", Parser::Reinitialize); diff --git a/src/node_trace_events.cc b/src/node_trace_events.cc new file mode 100644 index 00000000000..20edb66cd66 --- /dev/null +++ b/src/node_trace_events.cc @@ -0,0 +1,136 @@ +#include "node_internals.h" +#include "tracing/agent.h" + +namespace node { + +using v8::Context; +using v8::FunctionCallbackInfo; +using v8::Int32; +using v8::Local; +using v8::Object; +using v8::Value; + +// The tracing APIs require category groups to be pointers to long-lived +// strings. Those strings are stored here. +static std::unordered_set categoryGroups; + +// Gets a pointer to the category-enabled flags for a tracing category group, +// if tracing is enabled for it. +static const uint8_t* GetCategoryGroupEnabled(const char* category_group) { + if (category_group == nullptr) return nullptr; + + return TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(category_group); +} + +static const char* GetCategoryGroup(Environment* env, + const Local categoryValue) { + CHECK(categoryValue->IsString()); + + Utf8Value category(env->isolate(), categoryValue); + // If the value already exists in the set, insertion.first will point + // to the existing value. Thus, this will maintain a long lived pointer + // to the category c-string. + auto insertion = categoryGroups.insert(category.out()); + + // The returned insertion is a pair whose first item is the object that was + // inserted or that blocked the insertion and second item is a boolean + // indicating whether it was inserted. + return insertion.first->c_str(); +} + +static void Emit(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Local context = env->context(); + + // Args: [type, category, name, id, argName, argValue] + CHECK_GE(args.Length(), 3); + + // Check the category group first, to avoid doing more work if it's not + // enabled. + const char* category_group = GetCategoryGroup(env, args[1]); + const uint8_t* category_group_enabled = + GetCategoryGroupEnabled(category_group); + if (*category_group_enabled == 0) return; + + // get trace_event phase + CHECK(args[0]->IsNumber()); + char phase = static_cast(args[0]->Int32Value(context).ToChecked()); + + // get trace_event name + CHECK(args[2]->IsString()); + Utf8Value nameValue(env->isolate(), args[2]); + const char* name = nameValue.out(); + + // get trace_event id + int64_t id = 0; + bool has_id = false; + if (args.Length() >= 4 && !args[3]->IsUndefined() && !args[3]->IsNull()) { + has_id = true; + CHECK(args[3]->IsNumber()); + id = args[3]->IntegerValue(context).ToChecked(); + } + + // TODO(AndreasMadsen): Two extra arguments are not supported. + // TODO(AndreasMadsen): String values are not supported. + int32_t num_args = 0; + const char* arg_names[1]; + uint8_t arg_types[1]; + uint64_t arg_values[1]; + + // Create Utf8Value in the function scope to prevent deallocation. + // The c-string will be copied by TRACE_EVENT_API_ADD_TRACE_EVENT at the end. + Utf8Value arg1NameValue(env->isolate(), args[4]); + + if (args.Length() < 6 || (args[4]->IsUndefined() && args[5]->IsUndefined())) { + num_args = 0; + } else { + num_args = 1; + arg_types[0] = TRACE_VALUE_TYPE_INT; + + CHECK(args[4]->IsString()); + arg_names[0] = arg1NameValue.out(); + + CHECK(args[5]->IsNumber()); + arg_values[0] = args[5]->IntegerValue(context).ToChecked(); + } + + // The TRACE_EVENT_FLAG_COPY flag indicates that the name and argument + // name should be copied thus they don't need to long-lived pointers. + // The category name still won't be copied and thus need to be a long-lived + // pointer. + uint32_t flags = TRACE_EVENT_FLAG_COPY; + if (has_id) { + flags |= TRACE_EVENT_FLAG_HAS_ID; + } + + const char* scope = node::tracing::kGlobalScope; + uint64_t bind_id = node::tracing::kNoId; + + TRACE_EVENT_API_ADD_TRACE_EVENT( + phase, category_group_enabled, name, scope, id, bind_id, + num_args, arg_names, arg_types, arg_values, + flags); +} + +static void CategoryGroupEnabled(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + + const char* category_group = GetCategoryGroup(env, args[0]); + const uint8_t* category_group_enabled = + GetCategoryGroupEnabled(category_group); + args.GetReturnValue().Set(*category_group_enabled > 0); +} + +void InitializeTraceEvents(Local target, + Local unused, + Local context, + void* priv) { + Environment* env = Environment::GetCurrent(context); + + env->SetMethod(target, "emit", Emit); + env->SetMethod(target, "categoryGroupEnabled", CategoryGroupEnabled); +} + +} // namespace node + +NODE_MODULE_CONTEXT_AWARE_BUILTIN(trace_events, node::InitializeTraceEvents) diff --git a/src/tracing/agent.cc b/src/tracing/agent.cc index bc04b0b5e60..4514a0fce1f 100644 --- a/src/tracing/agent.cc +++ b/src/tracing/agent.cc @@ -36,6 +36,7 @@ void Agent::Start(const string& enabled_categories) { } else { trace_config->AddIncludedCategory("v8"); trace_config->AddIncludedCategory("node"); + trace_config->AddIncludedCategory("node.async_hooks"); } // This thread should be created *after* async handles are created diff --git a/test/parallel/test-trace-event.js b/test/parallel/test-trace-event.js deleted file mode 100644 index 434c7db4d26..00000000000 --- a/test/parallel/test-trace-event.js +++ /dev/null @@ -1,41 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const cp = require('child_process'); -const fs = require('fs'); - -const CODE = 'for (var i = 0; i < 100000; i++) { "test" + i }'; -const FILE_NAME = 'node_trace.1.log'; - -common.refreshTmpDir(); -process.chdir(common.tmpDir); - -const proc_no_categories = cp.spawn( - process.execPath, - [ '--trace-events-enabled', '--trace-event-categories', '""', '-e', CODE ] -); - -proc_no_categories.once('exit', common.mustCall(() => { - assert(!common.fileExists(FILE_NAME)); - - const proc = cp.spawn(process.execPath, - [ '--trace-events-enabled', '-e', CODE ]); - - proc.once('exit', common.mustCall(() => { - assert(common.fileExists(FILE_NAME)); - fs.readFile(FILE_NAME, common.mustCall((err, data) => { - const traces = JSON.parse(data.toString()).traceEvents; - assert(traces.length > 0); - // Values that should be present on all runs to approximate correctness. - assert(traces.some((trace) => { - if (trace.pid !== proc.pid) - return false; - if (trace.cat !== 'v8') - return false; - if (trace.name !== 'V8.ScriptCompiler') - return false; - return true; - })); - })); - })); -})); diff --git a/test/parallel/test-trace-events-all.js b/test/parallel/test-trace-events-all.js new file mode 100644 index 00000000000..329f99f5912 --- /dev/null +++ b/test/parallel/test-trace-events-all.js @@ -0,0 +1,56 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cp = require('child_process'); +const fs = require('fs'); + +const CODE = + 'setTimeout(() => { for (var i = 0; i < 100000; i++) { "test" + i } }, 1)'; +const FILE_NAME = 'node_trace.1.log'; + +common.refreshTmpDir(); +process.chdir(common.tmpDir); + +const proc = cp.spawn(process.execPath, + [ '--trace-events-enabled', '-e', CODE ]); + +proc.once('exit', common.mustCall(() => { + assert(common.fileExists(FILE_NAME)); + fs.readFile(FILE_NAME, common.mustCall((err, data) => { + const traces = JSON.parse(data.toString()).traceEvents; + assert(traces.length > 0); + // V8 trace events should be generated. + assert(traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'v8') + return false; + if (trace.name !== 'V8.ScriptCompiler') + return false; + return true; + })); + + // C++ async_hooks trace events should be generated. + assert(traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'node.async_hooks') + return false; + if (trace.name !== 'TIMERWRAP') + return false; + return true; + })); + + + // JavaScript async_hooks trace events should be generated. + assert(traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'node.async_hooks') + return false; + if (trace.name !== 'Timeout') + return false; + return true; + })); + })); +})); diff --git a/test/parallel/test-trace-events-async-hooks.js b/test/parallel/test-trace-events-async-hooks.js new file mode 100644 index 00000000000..7f8fb106200 --- /dev/null +++ b/test/parallel/test-trace-events-async-hooks.js @@ -0,0 +1,58 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cp = require('child_process'); +const fs = require('fs'); + +const CODE = + 'setTimeout(() => { for (var i = 0; i < 100000; i++) { "test" + i } }, 1)'; +const FILE_NAME = 'node_trace.1.log'; + +common.refreshTmpDir(); +process.chdir(common.tmpDir); + +const proc = cp.spawn(process.execPath, + [ '--trace-events-enabled', + '--trace-event-categories', 'node.async_hooks', + '-e', CODE ]); + +proc.once('exit', common.mustCall(() => { + assert(common.fileExists(FILE_NAME)); + fs.readFile(FILE_NAME, common.mustCall((err, data) => { + const traces = JSON.parse(data.toString()).traceEvents; + assert(traces.length > 0); + // V8 trace events should be generated. + assert(!traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'v8') + return false; + if (trace.name !== 'V8.ScriptCompiler') + return false; + return true; + })); + + // C++ async_hooks trace events should be generated. + assert(traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'node.async_hooks') + return false; + if (trace.name !== 'TIMERWRAP') + return false; + return true; + })); + + + // JavaScript async_hooks trace events should be generated. + assert(traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'node.async_hooks') + return false; + if (trace.name !== 'Timeout') + return false; + return true; + })); + })); +})); diff --git a/test/parallel/test-trace-events-binding.js b/test/parallel/test-trace-events-binding.js new file mode 100644 index 00000000000..628c9cace71 --- /dev/null +++ b/test/parallel/test-trace-events-binding.js @@ -0,0 +1,48 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cp = require('child_process'); +const fs = require('fs'); + +const CODE = ` + process.binding("trace_events").emit( + 'b'.charCodeAt(0), 'custom', + 'type-value', 10, 'extra-value', 20); + process.binding("trace_events").emit( + 'b'.charCodeAt(0), 'custom', + 'type-value', 20); + process.binding("trace_events").emit( + 'b'.charCodeAt(0), 'missing', + 'type-value', 10, 'extra-value', 20); +`; +const FILE_NAME = 'node_trace.1.log'; + +common.refreshTmpDir(); +process.chdir(common.tmpDir); + +const proc = cp.spawn(process.execPath, + [ '--trace-events-enabled', + '--trace-event-categories', 'custom', + '-e', CODE ]); + +proc.once('exit', common.mustCall(() => { + assert(common.fileExists(FILE_NAME)); + fs.readFile(FILE_NAME, common.mustCall((err, data) => { + const traces = JSON.parse(data.toString()).traceEvents; + assert.strictEqual(traces.length, 2); + + assert.strictEqual(traces[0].pid, proc.pid); + assert.strictEqual(traces[0].ph, 'b'); + assert.strictEqual(traces[0].cat, 'custom'); + assert.strictEqual(traces[0].name, 'type-value'); + assert.strictEqual(traces[0].id, '0xa'); + assert.deepStrictEqual(traces[0].args, { 'extra-value': 20 }); + + assert.strictEqual(traces[1].pid, proc.pid); + assert.strictEqual(traces[1].ph, 'b'); + assert.strictEqual(traces[1].cat, 'custom'); + assert.strictEqual(traces[1].name, 'type-value'); + assert.strictEqual(traces[1].id, '0x14'); + assert.deepStrictEqual(traces[1].args, { }); + })); +})); diff --git a/test/parallel/test-trace-events-category-used.js b/test/parallel/test-trace-events-category-used.js new file mode 100644 index 00000000000..39d09ad862d --- /dev/null +++ b/test/parallel/test-trace-events-category-used.js @@ -0,0 +1,35 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cp = require('child_process'); + +const CODE = `console.log( + process.binding("trace_events").categoryGroupEnabled("custom") +);`; + +common.refreshTmpDir(); +process.chdir(common.tmpDir); + +const procEnabled = cp.spawn( + process.execPath, + [ '--trace-events-enabled', '--trace-event-categories', 'custom', '-e', CODE ] +); +let procEnabledOutput = ''; + +procEnabled.stdout.on('data', (data) => procEnabledOutput += data); +procEnabled.stderr.pipe(process.stderr); +procEnabled.once('exit', common.mustCall(() => { + assert.strictEqual(procEnabledOutput, 'true\n'); +})); + +const procDisabled = cp.spawn( + process.execPath, + [ '--trace-events-enabled', '--trace-event-categories', 'other', '-e', CODE ] +); +let procDisabledOutput = ''; + +procDisabled.stdout.on('data', (data) => procDisabledOutput += data); +procDisabled.stderr.pipe(process.stderr); +procDisabled.once('exit', common.mustCall(() => { + assert.strictEqual(procDisabledOutput, 'false\n'); +})); diff --git a/test/parallel/test-trace-events-none.js b/test/parallel/test-trace-events-none.js new file mode 100644 index 00000000000..9a4d587f2db --- /dev/null +++ b/test/parallel/test-trace-events-none.js @@ -0,0 +1,20 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cp = require('child_process'); + +const CODE = + 'setTimeout(() => { for (var i = 0; i < 100000; i++) { "test" + i } }, 1)'; +const FILE_NAME = 'node_trace.1.log'; + +common.refreshTmpDir(); +process.chdir(common.tmpDir); + +const proc_no_categories = cp.spawn( + process.execPath, + [ '--trace-events-enabled', '--trace-event-categories', '""', '-e', CODE ] +); + +proc_no_categories.once('exit', common.mustCall(() => { + assert(!common.fileExists(FILE_NAME)); +})); diff --git a/test/parallel/test-trace-events-v8.js b/test/parallel/test-trace-events-v8.js new file mode 100644 index 00000000000..b17b1473eca --- /dev/null +++ b/test/parallel/test-trace-events-v8.js @@ -0,0 +1,58 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const cp = require('child_process'); +const fs = require('fs'); + +const CODE = + 'setTimeout(() => { for (var i = 0; i < 100000; i++) { "test" + i } }, 1)'; +const FILE_NAME = 'node_trace.1.log'; + +common.refreshTmpDir(); +process.chdir(common.tmpDir); + +const proc = cp.spawn(process.execPath, + [ '--trace-events-enabled', + '--trace-event-categories', 'v8', + '-e', CODE ]); + +proc.once('exit', common.mustCall(() => { + assert(common.fileExists(FILE_NAME)); + fs.readFile(FILE_NAME, common.mustCall((err, data) => { + const traces = JSON.parse(data.toString()).traceEvents; + assert(traces.length > 0); + // V8 trace events should be generated. + assert(traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'v8') + return false; + if (trace.name !== 'V8.ScriptCompiler') + return false; + return true; + })); + + // C++ async_hooks trace events should be generated. + assert(!traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'node.async_hooks') + return false; + if (trace.name !== 'TIMERWRAP') + return false; + return true; + })); + + + // JavaScript async_hooks trace events should be generated. + assert(!traces.some((trace) => { + if (trace.pid !== proc.pid) + return false; + if (trace.cat !== 'node.async_hooks') + return false; + if (trace.name !== 'Timeout') + return false; + return true; + })); + })); +})); From cb137eb15914a6436c55e8faeeb181021496be0d Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 7 Nov 2017 21:16:36 +0100 Subject: [PATCH 11/12] doc: document fs.realpath.native() Mea culpa, somehow I managed to drop the documentation commit while merging the pull request. This should have been included in commit 74023c072c ("fs: expose realpath(3) bindings") from this month. PR-URL: https://github.com/nodejs/node/pull/17059 Refs: https://github.com/nodejs/node/pull/15776 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Evan Lucas Reviewed-By: Luigi Pinca Reviewed-By: Refael Ackermann --- doc/api/fs.md | 79 ++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 75 insertions(+), 4 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index 06f889e47fb..c282b418d3b 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -2028,8 +2028,21 @@ changes: * `err` {Error} * `resolvedPath` {string|Buffer} -Asynchronous realpath(3). The `callback` gets two arguments `(err, -resolvedPath)`. May use `process.cwd` to resolve relative paths. +Asynchronously computes the canonical pathname by resolving `.`, `..` and +symbolic links. + +Note that "canonical" does not mean "unique": hard links and bind mounts can +expose a file system entity through many pathnames. + +This function behaves like realpath(3), with some exceptions: + +1. No case conversion is performed on case-insensitive file systems. + +2. The maximum number of symbolic links is platform-independent and generally + (much) higher than what the native realpath(3) implementation supports. + +The `callback` gets two arguments `(err, resolvedPath)`. May use `process.cwd` +to resolve relative paths. Only paths that can be converted to UTF8 strings are supported. @@ -2041,6 +2054,33 @@ the path returned will be passed as a `Buffer` object. *Note*: If `path` resolves to a socket or a pipe, the function will return a system dependent name for that object. +## fs.realpath.native(path[, options], callback) + + +* `path` {string|Buffer|URL} +* `options` {string|Object} + * `encoding` {string} **Default:** `'utf8'` +* `callback` {Function} + * `err` {Error} + * `resolvedPath` {string|Buffer} + +Asynchronous realpath(3). + +The `callback` gets two arguments `(err, resolvedPath)`. + +Only paths that can be converted to UTF8 strings are supported. + +The optional `options` argument can be a string specifying an encoding, or an +object with an `encoding` property specifying the character encoding to use for +the path passed to the callback. If the `encoding` is set to `'buffer'`, +the path returned will be passed as a `Buffer` object. + +*Note*: On Linux, when Node.js is linked against musl libc, the procfs file +system must be mounted on `/proc` in order for this function to work. Glibc +does not have this restriction. + ## fs.realpathSync(path[, options]) + +* `path` {string|Buffer|URL} +* `options` {string|Object} + * `encoding` {string} **Default:** `'utf8'` + +Synchronous realpath(3). + +Only paths that can be converted to UTF8 strings are supported. + +The optional `options` argument can be a string specifying an encoding, or an +object with an `encoding` property specifying the character encoding to use for +the path passed to the callback. If the `encoding` is set to `'buffer'`, +the path returned will be passed as a `Buffer` object. + +*Note*: On Linux, when Node.js is linked against musl libc, the procfs file +system must be mounted on `/proc` in order for this function to work. Glibc +does not have this restriction. + ## fs.rename(oldPath, newPath, callback)