From ad86384cd38ac9715ff040c3c72864084b4c3a59 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Thu, 24 May 2018 06:58:54 -0700 Subject: [PATCH 1/3] Revert "src: move *Exceptions out to separate cc/h" This reverts commit 9349e15daa803fc626581802fa827afe8d00340e. --- node.gyp | 2 - src/callback_scope.h | 11 ++- src/core.h | 44 --------- src/exceptions.cc | 215 ------------------------------------------- src/exceptions.h | 76 --------------- src/node.cc | 195 +++++++++++++++++++++++++++++++++++++++ src/node.h | 103 ++++++++++++++++++++- 7 files changed, 306 insertions(+), 340 deletions(-) delete mode 100644 src/core.h delete mode 100644 src/exceptions.cc delete mode 100644 src/exceptions.h diff --git a/node.gyp b/node.gyp index 038e5219bcc0fe..4a1aecd1fca86b 100644 --- a/node.gyp +++ b/node.gyp @@ -314,7 +314,6 @@ 'src/connection_wrap.cc', 'src/connect_wrap.cc', 'src/env.cc', - 'src/exceptions.cc', 'src/fs_event_wrap.cc', 'src/handle_wrap.cc', 'src/js_stream.cc', @@ -377,7 +376,6 @@ 'src/connect_wrap.h', 'src/env.h', 'src/env-inl.h', - 'src/exceptions.h', 'src/handle_wrap.h', 'src/js_stream.h', 'src/module_wrap.h', diff --git a/src/callback_scope.h b/src/callback_scope.h index 7f199aaea228d4..11fee0dd57ac81 100644 --- a/src/callback_scope.h +++ b/src/callback_scope.h @@ -1,7 +1,16 @@ #ifndef SRC_CALLBACK_SCOPE_H_ #define SRC_CALLBACK_SCOPE_H_ -#include "core.h" +#ifdef _WIN32 +# ifndef BUILDING_NODE_EXTENSION +# define NODE_EXTERN __declspec(dllexport) +# else +# define NODE_EXTERN __declspec(dllimport) +# endif +#else +# define NODE_EXTERN /* nothing */ +#endif + #include "v8.h" namespace node { diff --git a/src/core.h b/src/core.h deleted file mode 100644 index 0dbce4f3f8f6f9..00000000000000 --- a/src/core.h +++ /dev/null @@ -1,44 +0,0 @@ -#ifndef SRC_CORE_H_ -#define SRC_CORE_H_ - -#ifdef _WIN32 -# ifndef BUILDING_NODE_EXTENSION -# define NODE_EXTERN __declspec(dllexport) -# else -# define NODE_EXTERN __declspec(dllimport) -# endif -#else -# define NODE_EXTERN /* nothing */ -#endif - -#define NODE_MAKE_VERSION(major, minor, patch) \ - ((major) * 0x1000 + (minor) * 0x100 + (patch)) - -#ifdef __clang__ -# define NODE_CLANG_AT_LEAST(major, minor, patch) \ - (NODE_MAKE_VERSION(major, minor, patch) <= \ - NODE_MAKE_VERSION(__clang_major__, __clang_minor__, __clang_patchlevel__)) -#else -# define NODE_CLANG_AT_LEAST(major, minor, patch) (0) -#endif - -#ifdef __GNUC__ -# define NODE_GNUC_AT_LEAST(major, minor, patch) \ - (NODE_MAKE_VERSION(major, minor, patch) <= \ - NODE_MAKE_VERSION(__GNUC__, __GNUC_MINOR__, __GNUC_PATCHLEVEL__)) -#else -# define NODE_GNUC_AT_LEAST(major, minor, patch) (0) -#endif - -#if NODE_CLANG_AT_LEAST(2, 9, 0) || NODE_GNUC_AT_LEAST(4, 5, 0) -# define NODE_DEPRECATED(message, declarator) \ - __attribute__((deprecated(message))) declarator -#elif defined(_MSC_VER) -# define NODE_DEPRECATED(message, declarator) \ - __declspec(deprecated) declarator -#else -# define NODE_DEPRECATED(message, declarator) \ - declarator -#endif - -#endif // SRC_CORE_H_ diff --git a/src/exceptions.cc b/src/exceptions.cc deleted file mode 100644 index 3068483bdf58ff..00000000000000 --- a/src/exceptions.cc +++ /dev/null @@ -1,215 +0,0 @@ -#include "node.h" -#include "node_internals.h" -#include "env.h" -#include "env-inl.h" -#include "exceptions.h" -#include "util.h" -#include "util-inl.h" -#include "v8.h" -#include "uv.h" - -#include - -namespace node { - -using v8::Exception; -using v8::Integer; -using v8::Isolate; -using v8::Local; -using v8::Message; -using v8::Object; -using v8::String; -using v8::Value; - -Local ErrnoException(Isolate* isolate, - int errorno, - const char *syscall, - const char *msg, - const char *path) { - Environment* env = Environment::GetCurrent(isolate); - - Local e; - Local estring = OneByteString(env->isolate(), errno_string(errorno)); - if (msg == nullptr || msg[0] == '\0') { - msg = strerror(errorno); - } - Local message = OneByteString(env->isolate(), msg); - - Local cons = - String::Concat(estring, FIXED_ONE_BYTE_STRING(env->isolate(), ", ")); - cons = String::Concat(cons, message); - - Local path_string; - if (path != nullptr) { - // FIXME(bnoordhuis) It's questionable to interpret the file path as UTF-8. - path_string = String::NewFromUtf8(env->isolate(), path); - } - - if (path_string.IsEmpty() == false) { - cons = String::Concat(cons, FIXED_ONE_BYTE_STRING(env->isolate(), " '")); - cons = String::Concat(cons, path_string); - cons = String::Concat(cons, FIXED_ONE_BYTE_STRING(env->isolate(), "'")); - } - e = Exception::Error(cons); - - Local obj = e.As(); - obj->Set(env->errno_string(), Integer::New(env->isolate(), errorno)); - obj->Set(env->code_string(), estring); - - if (path_string.IsEmpty() == false) { - obj->Set(env->path_string(), path_string); - } - - if (syscall != nullptr) { - obj->Set(env->syscall_string(), OneByteString(env->isolate(), syscall)); - } - - return e; -} - -static Local StringFromPath(Isolate* isolate, const char* path) { -#ifdef _WIN32 - if (strncmp(path, "\\\\?\\UNC\\", 8) == 0) { - return String::Concat(FIXED_ONE_BYTE_STRING(isolate, "\\\\"), - String::NewFromUtf8(isolate, path + 8)); - } else if (strncmp(path, "\\\\?\\", 4) == 0) { - return String::NewFromUtf8(isolate, path + 4); - } -#endif - - return String::NewFromUtf8(isolate, path); -} - - -Local UVException(Isolate* isolate, - int errorno, - const char* syscall, - const char* msg, - const char* path) { - return UVException(isolate, errorno, syscall, msg, path, nullptr); -} - - -Local UVException(Isolate* isolate, - int errorno, - const char* syscall, - const char* msg, - const char* path, - const char* dest) { - Environment* env = Environment::GetCurrent(isolate); - - if (!msg || !msg[0]) - msg = uv_strerror(errorno); - - Local js_code = OneByteString(isolate, uv_err_name(errorno)); - Local js_syscall = OneByteString(isolate, syscall); - Local js_path; - Local js_dest; - - Local js_msg = js_code; - js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, ": ")); - js_msg = String::Concat(js_msg, OneByteString(isolate, msg)); - js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, ", ")); - js_msg = String::Concat(js_msg, js_syscall); - - if (path != nullptr) { - js_path = StringFromPath(isolate, path); - - js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, " '")); - js_msg = String::Concat(js_msg, js_path); - js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, "'")); - } - - if (dest != nullptr) { - js_dest = StringFromPath(isolate, dest); - - js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, " -> '")); - js_msg = String::Concat(js_msg, js_dest); - js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, "'")); - } - - Local e = Exception::Error(js_msg)->ToObject(isolate); - - e->Set(env->errno_string(), Integer::New(isolate, errorno)); - e->Set(env->code_string(), js_code); - e->Set(env->syscall_string(), js_syscall); - if (!js_path.IsEmpty()) - e->Set(env->path_string(), js_path); - if (!js_dest.IsEmpty()) - e->Set(env->dest_string(), js_dest); - - return e; -} - -#ifdef _WIN32 -// Does about the same as strerror(), -// but supports all windows error messages -static const char *winapi_strerror(const int errorno, bool* must_free) { - char *errmsg = nullptr; - - FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | - FORMAT_MESSAGE_IGNORE_INSERTS, nullptr, errorno, - MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (LPTSTR)&errmsg, 0, nullptr); - - if (errmsg) { - *must_free = true; - - // Remove trailing newlines - for (int i = strlen(errmsg) - 1; - i >= 0 && (errmsg[i] == '\n' || errmsg[i] == '\r'); i--) { - errmsg[i] = '\0'; - } - - return errmsg; - } else { - // FormatMessage failed - *must_free = false; - return "Unknown error"; - } -} - - -Local WinapiErrnoException(Isolate* isolate, - int errorno, - const char* syscall, - const char* msg, - const char* path) { - Environment* env = Environment::GetCurrent(isolate); - Local e; - bool must_free = false; - if (!msg || !msg[0]) { - msg = winapi_strerror(errorno, &must_free); - } - Local message = OneByteString(env->isolate(), msg); - - if (path) { - Local cons1 = - String::Concat(message, FIXED_ONE_BYTE_STRING(isolate, " '")); - Local cons2 = - String::Concat(cons1, String::NewFromUtf8(isolate, path)); - Local cons3 = - String::Concat(cons2, FIXED_ONE_BYTE_STRING(isolate, "'")); - e = Exception::Error(cons3); - } else { - e = Exception::Error(message); - } - - Local obj = e.As(); - obj->Set(env->errno_string(), Integer::New(isolate, errorno)); - - if (path != nullptr) { - obj->Set(env->path_string(), String::NewFromUtf8(isolate, path)); - } - - if (syscall != nullptr) { - obj->Set(env->syscall_string(), OneByteString(isolate, syscall)); - } - - if (must_free) - LocalFree((HLOCAL)msg); - - return e; -} -#endif - -} // namespace node diff --git a/src/exceptions.h b/src/exceptions.h deleted file mode 100644 index 8a4a8ce99e876d..00000000000000 --- a/src/exceptions.h +++ /dev/null @@ -1,76 +0,0 @@ -#ifndef SRC_EXCEPTIONS_H_ -#define SRC_EXCEPTIONS_H_ - -#include "core.h" -#include "v8.h" - -namespace node { - -NODE_EXTERN v8::Local ErrnoException(v8::Isolate* isolate, - int errorno, - const char* syscall = nullptr, - const char* message = nullptr, - const char* path = nullptr); -NODE_EXTERN v8::Local UVException(v8::Isolate* isolate, - int errorno, - const char* syscall = nullptr, - const char* message = nullptr, - const char* path = nullptr); -NODE_EXTERN v8::Local UVException(v8::Isolate* isolate, - int errorno, - const char* syscall, - const char* message, - const char* path, - const char* dest); - -NODE_DEPRECATED( - "Use ErrnoException(isolate, ...)", - inline v8::Local ErrnoException( - int errorno, - const char* syscall = nullptr, - const char* message = nullptr, - const char* path = nullptr) { - return ErrnoException(v8::Isolate::GetCurrent(), - errorno, - syscall, - message, - path); -}) - -inline v8::Local UVException(int errorno, - const char* syscall = nullptr, - const char* message = nullptr, - const char* path = nullptr) { - return UVException(v8::Isolate::GetCurrent(), - errorno, - syscall, - message, - path); -} - -#ifdef _WIN32 -NODE_EXTERN v8::Local WinapiErrnoException( - v8::Isolate* isolate, - int errorno, - const char *syscall = nullptr, - const char *msg = "", - const char *path = nullptr); - -NODE_DEPRECATED( - "Use WinapiErrnoException(isolate, ...)", - inline v8::Local WinapiErrnoException( - int errorno, - const char *syscall = nullptr, - const char *msg = "", - const char *path = nullptr) { - return WinapiErrnoException(v8::Isolate::GetCurrent(), - errorno, - syscall, - msg, - path); -}) -#endif - -} // namespace node - -#endif // SRC_EXCEPTIONS_H_ diff --git a/src/node.cc b/src/node.cc index 86f4b91b59833e..21d8486cac7876 100644 --- a/src/node.cc +++ b/src/node.cc @@ -576,6 +576,129 @@ const char *signo_string(int signo) { } } + +Local ErrnoException(Isolate* isolate, + int errorno, + const char *syscall, + const char *msg, + const char *path) { + Environment* env = Environment::GetCurrent(isolate); + + Local e; + Local estring = OneByteString(env->isolate(), errno_string(errorno)); + if (msg == nullptr || msg[0] == '\0') { + msg = strerror(errorno); + } + Local message = OneByteString(env->isolate(), msg); + + Local cons = + String::Concat(estring, FIXED_ONE_BYTE_STRING(env->isolate(), ", ")); + cons = String::Concat(cons, message); + + Local path_string; + if (path != nullptr) { + // FIXME(bnoordhuis) It's questionable to interpret the file path as UTF-8. + path_string = String::NewFromUtf8(env->isolate(), path); + } + + if (path_string.IsEmpty() == false) { + cons = String::Concat(cons, FIXED_ONE_BYTE_STRING(env->isolate(), " '")); + cons = String::Concat(cons, path_string); + cons = String::Concat(cons, FIXED_ONE_BYTE_STRING(env->isolate(), "'")); + } + e = Exception::Error(cons); + + Local obj = e.As(); + obj->Set(env->errno_string(), Integer::New(env->isolate(), errorno)); + obj->Set(env->code_string(), estring); + + if (path_string.IsEmpty() == false) { + obj->Set(env->path_string(), path_string); + } + + if (syscall != nullptr) { + obj->Set(env->syscall_string(), OneByteString(env->isolate(), syscall)); + } + + return e; +} + + +static Local StringFromPath(Isolate* isolate, const char* path) { +#ifdef _WIN32 + if (strncmp(path, "\\\\?\\UNC\\", 8) == 0) { + return String::Concat(FIXED_ONE_BYTE_STRING(isolate, "\\\\"), + String::NewFromUtf8(isolate, path + 8)); + } else if (strncmp(path, "\\\\?\\", 4) == 0) { + return String::NewFromUtf8(isolate, path + 4); + } +#endif + + return String::NewFromUtf8(isolate, path); +} + + +Local UVException(Isolate* isolate, + int errorno, + const char* syscall, + const char* msg, + const char* path) { + return UVException(isolate, errorno, syscall, msg, path, nullptr); +} + + +Local UVException(Isolate* isolate, + int errorno, + const char* syscall, + const char* msg, + const char* path, + const char* dest) { + Environment* env = Environment::GetCurrent(isolate); + + if (!msg || !msg[0]) + msg = uv_strerror(errorno); + + Local js_code = OneByteString(isolate, uv_err_name(errorno)); + Local js_syscall = OneByteString(isolate, syscall); + Local js_path; + Local js_dest; + + Local js_msg = js_code; + js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, ": ")); + js_msg = String::Concat(js_msg, OneByteString(isolate, msg)); + js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, ", ")); + js_msg = String::Concat(js_msg, js_syscall); + + if (path != nullptr) { + js_path = StringFromPath(isolate, path); + + js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, " '")); + js_msg = String::Concat(js_msg, js_path); + js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, "'")); + } + + if (dest != nullptr) { + js_dest = StringFromPath(isolate, dest); + + js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, " -> '")); + js_msg = String::Concat(js_msg, js_dest); + js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, "'")); + } + + Local e = Exception::Error(js_msg)->ToObject(isolate); + + e->Set(env->errno_string(), Integer::New(isolate, errorno)); + e->Set(env->code_string(), js_code); + e->Set(env->syscall_string(), js_syscall); + if (!js_path.IsEmpty()) + e->Set(env->path_string(), js_path); + if (!js_dest.IsEmpty()) + e->Set(env->dest_string(), js_dest); + + return e; +} + + // Look up environment variable unless running as setuid root. bool SafeGetenv(const char* key, std::string* text) { #if !defined(__CloudABI__) && !defined(_WIN32) @@ -597,6 +720,78 @@ bool SafeGetenv(const char* key, std::string* text) { } +#ifdef _WIN32 +// Does about the same as strerror(), +// but supports all windows error messages +static const char *winapi_strerror(const int errorno, bool* must_free) { + char *errmsg = nullptr; + + FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | + FORMAT_MESSAGE_IGNORE_INSERTS, nullptr, errorno, + MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (LPTSTR)&errmsg, 0, nullptr); + + if (errmsg) { + *must_free = true; + + // Remove trailing newlines + for (int i = strlen(errmsg) - 1; + i >= 0 && (errmsg[i] == '\n' || errmsg[i] == '\r'); i--) { + errmsg[i] = '\0'; + } + + return errmsg; + } else { + // FormatMessage failed + *must_free = false; + return "Unknown error"; + } +} + + +Local WinapiErrnoException(Isolate* isolate, + int errorno, + const char* syscall, + const char* msg, + const char* path) { + Environment* env = Environment::GetCurrent(isolate); + Local e; + bool must_free = false; + if (!msg || !msg[0]) { + msg = winapi_strerror(errorno, &must_free); + } + Local message = OneByteString(env->isolate(), msg); + + if (path) { + Local cons1 = + String::Concat(message, FIXED_ONE_BYTE_STRING(isolate, " '")); + Local cons2 = + String::Concat(cons1, String::NewFromUtf8(isolate, path)); + Local cons3 = + String::Concat(cons2, FIXED_ONE_BYTE_STRING(isolate, "'")); + e = Exception::Error(cons3); + } else { + e = Exception::Error(message); + } + + Local obj = e.As(); + obj->Set(env->errno_string(), Integer::New(isolate, errorno)); + + if (path != nullptr) { + obj->Set(env->path_string(), String::NewFromUtf8(isolate, path)); + } + + if (syscall != nullptr) { + obj->Set(env->syscall_string(), OneByteString(isolate, syscall)); + } + + if (must_free) + LocalFree((HLOCAL)msg); + + return e; +} +#endif + + void* ArrayBufferAllocator::Allocate(size_t size) { if (zero_fill_field_ || zero_fill_all_buffers) return UncheckedCalloc(size); diff --git a/src/node.h b/src/node.h index d4b934645163f1..e10e393cd64d4b 100644 --- a/src/node.h +++ b/src/node.h @@ -22,6 +22,16 @@ #ifndef SRC_NODE_H_ #define SRC_NODE_H_ +#ifdef _WIN32 +# ifndef BUILDING_NODE_EXTENSION +# define NODE_EXTERN __declspec(dllexport) +# else +# define NODE_EXTERN __declspec(dllimport) +# endif +#else +# define NODE_EXTERN /* nothing */ +#endif + #ifdef BUILDING_NODE_EXTENSION # undef BUILDING_V8_SHARED # undef BUILDING_UV_SHARED @@ -50,12 +60,40 @@ # define SIGKILL 9 #endif -#include "core.h" // NOLINT(build/include_order) #include "v8.h" // NOLINT(build/include_order) #include "v8-platform.h" // NOLINT(build/include_order) #include "node_version.h" // NODE_MODULE_VERSION #include "callback_scope.h" -#include "exceptions.h" + +#define NODE_MAKE_VERSION(major, minor, patch) \ + ((major) * 0x1000 + (minor) * 0x100 + (patch)) + +#ifdef __clang__ +# define NODE_CLANG_AT_LEAST(major, minor, patch) \ + (NODE_MAKE_VERSION(major, minor, patch) <= \ + NODE_MAKE_VERSION(__clang_major__, __clang_minor__, __clang_patchlevel__)) +#else +# define NODE_CLANG_AT_LEAST(major, minor, patch) (0) +#endif + +#ifdef __GNUC__ +# define NODE_GNUC_AT_LEAST(major, minor, patch) \ + (NODE_MAKE_VERSION(major, minor, patch) <= \ + NODE_MAKE_VERSION(__GNUC__, __GNUC_MINOR__, __GNUC_PATCHLEVEL__)) +#else +# define NODE_GNUC_AT_LEAST(major, minor, patch) (0) +#endif + +#if NODE_CLANG_AT_LEAST(2, 9, 0) || NODE_GNUC_AT_LEAST(4, 5, 0) +# define NODE_DEPRECATED(message, declarator) \ + __attribute__((deprecated(message))) declarator +#elif defined(_MSC_VER) +# define NODE_DEPRECATED(message, declarator) \ + __declspec(deprecated) declarator +#else +# define NODE_DEPRECATED(message, declarator) \ + declarator +#endif // Forward-declare libuv loop struct uv_loop_s; @@ -69,6 +107,47 @@ class TracingController; // terminally confused when it's done in node_internals.h namespace node { +NODE_EXTERN v8::Local ErrnoException(v8::Isolate* isolate, + int errorno, + const char* syscall = nullptr, + const char* message = nullptr, + const char* path = nullptr); +NODE_EXTERN v8::Local UVException(v8::Isolate* isolate, + int errorno, + const char* syscall = nullptr, + const char* message = nullptr, + const char* path = nullptr); +NODE_EXTERN v8::Local UVException(v8::Isolate* isolate, + int errorno, + const char* syscall, + const char* message, + const char* path, + const char* dest); + +NODE_DEPRECATED("Use ErrnoException(isolate, ...)", + inline v8::Local ErrnoException( + int errorno, + const char* syscall = nullptr, + const char* message = nullptr, + const char* path = nullptr) { + return ErrnoException(v8::Isolate::GetCurrent(), + errorno, + syscall, + message, + path); +}) + +inline v8::Local UVException(int errorno, + const char* syscall = nullptr, + const char* message = nullptr, + const char* path = nullptr) { + return UVException(v8::Isolate::GetCurrent(), + errorno, + syscall, + message, + path); +} + /* * These methods need to be called in a HandleScope. * @@ -373,6 +452,26 @@ NODE_DEPRECATED("Use DecodeWrite(isolate, ...)", return DecodeWrite(v8::Isolate::GetCurrent(), buf, buflen, val, encoding); }) +#ifdef _WIN32 +NODE_EXTERN v8::Local WinapiErrnoException( + v8::Isolate* isolate, + int errorno, + const char *syscall = nullptr, + const char *msg = "", + const char *path = nullptr); + +NODE_DEPRECATED("Use WinapiErrnoException(isolate, ...)", + inline v8::Local WinapiErrnoException(int errorno, + const char *syscall = nullptr, const char *msg = "", + const char *path = nullptr) { + return WinapiErrnoException(v8::Isolate::GetCurrent(), + errorno, + syscall, + msg, + path); +}) +#endif + const char *signo_string(int errorno); From e697ee6f9edcf95135bdc3fab769f0feaf13429b Mon Sep 17 00:00:00 2001 From: James M Snell Date: Thu, 24 May 2018 06:59:04 -0700 Subject: [PATCH 2/3] Revert "src: fix odd linting issue" This reverts commit c68e6e1f1b4c70bf60485d20b6d4c9778dbbc551. --- src/node.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/node.cc b/src/node.cc index 21d8486cac7876..e99083c4904b15 100644 --- a/src/node.cc +++ b/src/node.cc @@ -2063,8 +2063,7 @@ node_module* get_linked_module(const char* name) { return FindModule(modlist_linked, name, NM_F_LINKED); } -class DLib { - public: +struct DLib { #ifdef __POSIX__ static const int kDefaultFlags = RTLD_LAZY; #else @@ -2085,7 +2084,7 @@ class DLib { #ifndef __POSIX__ uv_lib_t lib_; #endif - private: + DISALLOW_COPY_AND_ASSIGN(DLib); }; From 435e6496dc82016cc4ae6b70bc7405a5e867b7c0 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Thu, 24 May 2018 06:59:12 -0700 Subject: [PATCH 3/3] Revert "src: move CallbackScope to separate cc/h" This reverts commit fbd1c4977ed963e2aaebee871d3be4b0a090cc18. --- node.gyp | 2 - src/callback_scope.cc | 126 ------------------------------------------ src/callback_scope.h | 56 ------------------- src/node.cc | 111 +++++++++++++++++++++++++++++++++++++ src/node.h | 37 ++++++++++++- 5 files changed, 147 insertions(+), 185 deletions(-) delete mode 100644 src/callback_scope.cc delete mode 100644 src/callback_scope.h diff --git a/node.gyp b/node.gyp index 4a1aecd1fca86b..2cc76f0b0a0187 100644 --- a/node.gyp +++ b/node.gyp @@ -309,7 +309,6 @@ 'sources': [ 'src/async_wrap.cc', - 'src/callback_scope.cc', 'src/cares_wrap.cc', 'src/connection_wrap.cc', 'src/connect_wrap.cc', @@ -371,7 +370,6 @@ 'src/async_wrap-inl.h', 'src/base_object.h', 'src/base_object-inl.h', - 'src/callback_scope.h', 'src/connection_wrap.h', 'src/connect_wrap.h', 'src/env.h', diff --git a/src/callback_scope.cc b/src/callback_scope.cc deleted file mode 100644 index 5539d77c70cb4e..00000000000000 --- a/src/callback_scope.cc +++ /dev/null @@ -1,126 +0,0 @@ -#include "node.h" -#include "callback_scope.h" -#include "async_wrap.h" -#include "async_wrap-inl.h" -#include "env.h" -#include "env-inl.h" -#include "v8.h" - -namespace node { - -using v8::HandleScope; -using v8::Isolate; -using v8::Local; -using v8::Object; - -using AsyncHooks = Environment::AsyncHooks; - -CallbackScope::CallbackScope(Isolate* isolate, - Local object, - async_context asyncContext) - : private_(new InternalCallbackScope(Environment::GetCurrent(isolate), - object, - asyncContext)), - try_catch_(isolate) { - try_catch_.SetVerbose(true); -} - -CallbackScope::~CallbackScope() { - if (try_catch_.HasCaught()) - private_->MarkAsFailed(); - delete private_; -} - -InternalCallbackScope::InternalCallbackScope(AsyncWrap* async_wrap) - : InternalCallbackScope(async_wrap->env(), - async_wrap->object(), - { async_wrap->get_async_id(), - async_wrap->get_trigger_async_id() }) {} - -InternalCallbackScope::InternalCallbackScope(Environment* env, - Local object, - const async_context& asyncContext, - ResourceExpectation expect) - : env_(env), - async_context_(asyncContext), - object_(object), - callback_scope_(env) { - if (expect == kRequireResource) { - CHECK(!object.IsEmpty()); - } - - if (!env->can_call_into_js()) { - failed_ = true; - return; - } - - HandleScope handle_scope(env->isolate()); - // If you hit this assertion, you forgot to enter the v8::Context first. - CHECK_EQ(Environment::GetCurrent(env->isolate()), env); - - if (asyncContext.async_id != 0) { - // No need to check a return value because the application will exit if - // an exception occurs. - AsyncWrap::EmitBefore(env, asyncContext.async_id); - } - - if (!IsInnerMakeCallback()) { - env->tick_info()->set_has_thrown(false); - } - - env->async_hooks()->push_async_ids(async_context_.async_id, - async_context_.trigger_async_id); - pushed_ids_ = true; -} - -InternalCallbackScope::~InternalCallbackScope() { - Close(); -} - -void InternalCallbackScope::Close() { - if (closed_) return; - closed_ = true; - HandleScope handle_scope(env_->isolate()); - - if (pushed_ids_) - env_->async_hooks()->pop_async_id(async_context_.async_id); - - if (failed_) return; - - if (async_context_.async_id != 0) { - AsyncWrap::EmitAfter(env_, async_context_.async_id); - } - - if (IsInnerMakeCallback()) { - return; - } - - Environment::TickInfo* tick_info = env_->tick_info(); - - if (!env_->can_call_into_js()) return; - if (!tick_info->has_scheduled()) { - env_->isolate()->RunMicrotasks(); - } - - // Make sure the stack unwound properly. If there are nested MakeCallback's - // then it should return early and not reach this code. - if (env_->async_hooks()->fields()[AsyncHooks::kTotals]) { - CHECK_EQ(env_->execution_async_id(), 0); - CHECK_EQ(env_->trigger_async_id(), 0); - } - - if (!tick_info->has_scheduled() && !tick_info->has_promise_rejections()) { - return; - } - - Local process = env_->process_object(); - - if (!env_->can_call_into_js()) return; - - if (env_->tick_callback_function()->Call(process, 0, nullptr).IsEmpty()) { - env_->tick_info()->set_has_thrown(true); - failed_ = true; - } -} - -} // namespace node diff --git a/src/callback_scope.h b/src/callback_scope.h deleted file mode 100644 index 11fee0dd57ac81..00000000000000 --- a/src/callback_scope.h +++ /dev/null @@ -1,56 +0,0 @@ -#ifndef SRC_CALLBACK_SCOPE_H_ -#define SRC_CALLBACK_SCOPE_H_ - -#ifdef _WIN32 -# ifndef BUILDING_NODE_EXTENSION -# define NODE_EXTERN __declspec(dllexport) -# else -# define NODE_EXTERN __declspec(dllimport) -# endif -#else -# define NODE_EXTERN /* nothing */ -#endif - -#include "v8.h" - -namespace node { - -typedef double async_id; -struct async_context { - ::node::async_id async_id; - ::node::async_id trigger_async_id; -}; - -class InternalCallbackScope; - -/* This class works like `MakeCallback()` in that it sets up a specific - * asyncContext as the current one and informs the async_hooks and domains - * modules that this context is currently active. - * - * `MakeCallback()` is a wrapper around this class as well as - * `Function::Call()`. Either one of these mechanisms needs to be used for - * top-level calls into JavaScript (i.e. without any existing JS stack). - * - * This object should be stack-allocated to ensure that it is contained in a - * valid HandleScope. - */ -class NODE_EXTERN CallbackScope { - public: - CallbackScope(v8::Isolate* isolate, - v8::Local resource, - async_context asyncContext); - ~CallbackScope(); - - private: - InternalCallbackScope* private_; - v8::TryCatch try_catch_; - - void operator=(const CallbackScope&) = delete; - void operator=(CallbackScope&&) = delete; - CallbackScope(const CallbackScope&) = delete; - CallbackScope(CallbackScope&&) = delete; -}; - -} // namespace node - -#endif // SRC_CALLBACK_SCOPE_H_ diff --git a/src/node.cc b/src/node.cc index e99083c4904b15..fbca2f4f543d9c 100644 --- a/src/node.cc +++ b/src/node.cc @@ -170,6 +170,8 @@ using v8::Undefined; using v8::V8; using v8::Value; +using AsyncHooks = Environment::AsyncHooks; + static Mutex process_mutex; static Mutex environ_mutex; @@ -922,6 +924,115 @@ void RemoveEnvironmentCleanupHook(v8::Isolate* isolate, env->RemoveCleanupHook(fun, arg); } + +CallbackScope::CallbackScope(Isolate* isolate, + Local object, + async_context asyncContext) + : private_(new InternalCallbackScope(Environment::GetCurrent(isolate), + object, + asyncContext)), + try_catch_(isolate) { + try_catch_.SetVerbose(true); +} + +CallbackScope::~CallbackScope() { + if (try_catch_.HasCaught()) + private_->MarkAsFailed(); + delete private_; +} + +InternalCallbackScope::InternalCallbackScope(AsyncWrap* async_wrap) + : InternalCallbackScope(async_wrap->env(), + async_wrap->object(), + { async_wrap->get_async_id(), + async_wrap->get_trigger_async_id() }) {} + +InternalCallbackScope::InternalCallbackScope(Environment* env, + Local object, + const async_context& asyncContext, + ResourceExpectation expect) + : env_(env), + async_context_(asyncContext), + object_(object), + callback_scope_(env) { + if (expect == kRequireResource) { + CHECK(!object.IsEmpty()); + } + + if (!env->can_call_into_js()) { + failed_ = true; + return; + } + + HandleScope handle_scope(env->isolate()); + // If you hit this assertion, you forgot to enter the v8::Context first. + CHECK_EQ(Environment::GetCurrent(env->isolate()), env); + + if (asyncContext.async_id != 0) { + // No need to check a return value because the application will exit if + // an exception occurs. + AsyncWrap::EmitBefore(env, asyncContext.async_id); + } + + if (!IsInnerMakeCallback()) { + env->tick_info()->set_has_thrown(false); + } + + env->async_hooks()->push_async_ids(async_context_.async_id, + async_context_.trigger_async_id); + pushed_ids_ = true; +} + +InternalCallbackScope::~InternalCallbackScope() { + Close(); +} + +void InternalCallbackScope::Close() { + if (closed_) return; + closed_ = true; + HandleScope handle_scope(env_->isolate()); + + if (pushed_ids_) + env_->async_hooks()->pop_async_id(async_context_.async_id); + + if (failed_) return; + + if (async_context_.async_id != 0) { + AsyncWrap::EmitAfter(env_, async_context_.async_id); + } + + if (IsInnerMakeCallback()) { + return; + } + + Environment::TickInfo* tick_info = env_->tick_info(); + + if (!env_->can_call_into_js()) return; + if (!tick_info->has_scheduled()) { + env_->isolate()->RunMicrotasks(); + } + + // Make sure the stack unwound properly. If there are nested MakeCallback's + // then it should return early and not reach this code. + if (env_->async_hooks()->fields()[AsyncHooks::kTotals]) { + CHECK_EQ(env_->execution_async_id(), 0); + CHECK_EQ(env_->trigger_async_id(), 0); + } + + if (!tick_info->has_scheduled() && !tick_info->has_promise_rejections()) { + return; + } + + Local process = env_->process_object(); + + if (!env_->can_call_into_js()) return; + + if (env_->tick_callback_function()->Call(process, 0, nullptr).IsEmpty()) { + env_->tick_info()->set_has_thrown(true); + failed_ = true; + } +} + MaybeLocal InternalMakeCallback(Environment* env, Local recv, const Local callback, diff --git a/src/node.h b/src/node.h index e10e393cd64d4b..44c89afdb7134c 100644 --- a/src/node.h +++ b/src/node.h @@ -63,7 +63,6 @@ #include "v8.h" // NOLINT(build/include_order) #include "v8-platform.h" // NOLINT(build/include_order) #include "node_version.h" // NODE_MODULE_VERSION -#include "callback_scope.h" #define NODE_MAKE_VERSION(major, minor, patch) \ ((major) * 0x1000 + (minor) * 0x100 + (patch)) @@ -594,6 +593,12 @@ typedef void (*promise_hook_func) (v8::PromiseHookType type, v8::Local parent, void* arg); +typedef double async_id; +struct async_context { + ::node::async_id async_id; + ::node::async_id trigger_async_id; +}; + /* Registers an additional v8::PromiseHook wrapper. This API exists because V8 * itself supports only a single PromiseHook. */ NODE_EXTERN void AddPromiseHook(v8::Isolate* isolate, @@ -642,6 +647,36 @@ NODE_EXTERN async_context EmitAsyncInit(v8::Isolate* isolate, NODE_EXTERN void EmitAsyncDestroy(v8::Isolate* isolate, async_context asyncContext); +class InternalCallbackScope; + +/* This class works like `MakeCallback()` in that it sets up a specific + * asyncContext as the current one and informs the async_hooks and domains + * modules that this context is currently active. + * + * `MakeCallback()` is a wrapper around this class as well as + * `Function::Call()`. Either one of these mechanisms needs to be used for + * top-level calls into JavaScript (i.e. without any existing JS stack). + * + * This object should be stack-allocated to ensure that it is contained in a + * valid HandleScope. + */ +class NODE_EXTERN CallbackScope { + public: + CallbackScope(v8::Isolate* isolate, + v8::Local resource, + async_context asyncContext); + ~CallbackScope(); + + private: + InternalCallbackScope* private_; + v8::TryCatch try_catch_; + + void operator=(const CallbackScope&) = delete; + void operator=(CallbackScope&&) = delete; + CallbackScope(const CallbackScope&) = delete; + CallbackScope(CallbackScope&&) = delete; +}; + /* An API specific to emit before/after callbacks is unnecessary because * MakeCallback will automatically call them for you. *