diff --git a/node.gyp b/node.gyp index 911e3218aa..313326eb3f 100644 --- a/node.gyp +++ b/node.gyp @@ -223,7 +223,6 @@ 'src/signal_wrap.cc', 'src/spawn_sync.cc', 'src/string_bytes.cc', - 'src/string_search.cc', 'src/stream_base.cc', 'src/stream_wrap.cc', 'src/tcp_wrap.cc', @@ -687,7 +686,6 @@ '<(OBJ_PATH)<(OBJ_SEPARATOR)node_url.<(OBJ_SUFFIX)', '<(OBJ_PATH)<(OBJ_SEPARATOR)util.<(OBJ_SUFFIX)', '<(OBJ_PATH)<(OBJ_SEPARATOR)string_bytes.<(OBJ_SUFFIX)', - '<(OBJ_PATH)<(OBJ_SEPARATOR)string_search.<(OBJ_SUFFIX)', '<(OBJ_PATH)<(OBJ_SEPARATOR)stream_base.<(OBJ_SUFFIX)', '<(OBJ_PATH)<(OBJ_SEPARATOR)node_constants.<(OBJ_SUFFIX)', '<(OBJ_TRACING_PATH)<(OBJ_SEPARATOR)agent.<(OBJ_SUFFIX)', diff --git a/src/async-wrap-inl.h b/src/async-wrap-inl.h index ed5a0c0d27..4fcfe2e757 100644 --- a/src/async-wrap-inl.h +++ b/src/async-wrap-inl.h @@ -50,8 +50,13 @@ inline v8::MaybeLocal AsyncWrap::MakeCallback( const v8::Local symbol, int argc, v8::Local* argv) { - v8::Local cb_v = object()->Get(symbol); - CHECK(cb_v->IsFunction()); + v8::Local cb_v; + if (!object()->Get(object()->CreationContext(), symbol).ToLocal(&cb_v)) + return v8::MaybeLocal(); + if (!cb_v->IsFunction()) { + env()->ThrowError("callback must be a function"); + return v8::MaybeLocal(); + } return MakeCallback(cb_v.As(), argc, argv); } @@ -60,8 +65,13 @@ inline v8::MaybeLocal AsyncWrap::MakeCallback( uint32_t index, int argc, v8::Local* argv) { - v8::Local cb_v = object()->Get(index); - CHECK(cb_v->IsFunction()); + v8::Local cb_v; + if (!object()->Get(object()->CreationContext(), index).ToLocal(&cb_v)) + return v8::MaybeLocal(); + if (!cb_v->IsFunction()) { + env()->ThrowError("callback must be a function"); + return v8::MaybeLocal(); + } return MakeCallback(cb_v.As(), argc, argv); } diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 9b20d1c42b..404c51c6af 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -152,6 +152,7 @@ static void DestroyIdsCb(uv_timer_t* handle) { do { std::vector destroy_ids_list; destroy_ids_list.swap(*env->destroy_ids_list()); + if (!env->can_call_into_js()) return; for (auto current_id : destroy_ids_list) { // Want each callback to be cleaned up after itself, instead of cleaning // them all up after the while() loop completes. @@ -174,6 +175,9 @@ static void PushBackDestroyId(Environment* env, double id) { if (env->async_hooks()->fields()[AsyncHooks::kDestroy] == 0) return; + if (!env->can_call_into_js()) + return; + if (env->destroy_ids_list()->empty()) uv_timer_start(env->destroy_ids_timer_handle(), DestroyIdsCb, 0, 0); diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index e800e0f2fe..c26d18bd8e 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -70,6 +70,8 @@ using v8::Value; namespace { +Mutex ares_library_mutex; + inline uint16_t cares_get_16bit(const unsigned char* p) { return static_cast(p[0] << 8U) | (static_cast(p[1])); } @@ -496,6 +498,7 @@ void ChannelWrap::Setup() { int r; if (!library_inited_) { + Mutex::ScopedLock lock(ares_library_mutex); // Multiple calls to ares_library_init() increase a reference counter, // so this is a no-op except for the first call to it. r = ares_library_init(ARES_LIB_INIT_ALL); @@ -509,6 +512,7 @@ void ChannelWrap::Setup() { ARES_OPT_FLAGS | ARES_OPT_SOCK_STATE_CB); if (r != ARES_SUCCESS) { + Mutex::ScopedLock lock(ares_library_mutex); ares_library_cleanup(); return env()->ThrowError(ToErrorCodeString(r)); } @@ -526,6 +530,7 @@ void ChannelWrap::Setup() { ChannelWrap::~ChannelWrap() { if (library_inited_) { + Mutex::ScopedLock lock(ares_library_mutex); // This decreases the reference counter increased by ares_library_init(). ares_library_cleanup(); } diff --git a/src/env-inl.h b/src/env-inl.h index 28512003f0..7209e9458a 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -332,6 +332,7 @@ inline Environment::~Environment() { delete[] heap_space_statistics_buffer_; delete[] http_parser_buffer_; delete http2_state_; + delete[] fs_stats_field_array_; free(performance_state_); } @@ -503,6 +504,14 @@ inline void Environment::set_fs_stats_field_array(double* fields) { fs_stats_field_array_ = fields; } +inline bool Environment::can_call_into_js() const { + return can_call_into_js_; +} + +inline void Environment::set_can_call_into_js(bool can_call_into_js) { + can_call_into_js_ = can_call_into_js; +} + inline performance::performance_state* Environment::performance_state() { return performance_state_; } @@ -596,6 +605,26 @@ inline void Environment::SetTemplateMethod(v8::Local that, t->SetClassName(name_string); // NODE_SET_METHOD() compatibility. } +void Environment::AddCleanupHook(void (*fn)(void*), void* arg) { + cleanup_hooks_[arg].push_back( + CleanupHookCallback { fn, arg, cleanup_hook_counter_++ }); +} + +void Environment::RemoveCleanupHook(void (*fn)(void*), void* arg) { + auto map_it = cleanup_hooks_.find(arg); + if (map_it == cleanup_hooks_.end()) + return; + + for (auto it = map_it->second.begin(); it != map_it->second.end(); ++it) { + if (it->fun_ == fn && it->arg_ == arg) { + map_it->second.erase(it); + if (map_it->second.empty()) + cleanup_hooks_.erase(arg); + return; + } + } +} + #define VP(PropertyName, StringValue) V(v8::Private, PropertyName) #define VS(PropertyName, StringValue) V(v8::String, PropertyName) #define V(TypeName, PropertyName) \ diff --git a/src/env.cc b/src/env.cc index 25d80ba1e8..e7f7929b7d 100644 --- a/src/env.cc +++ b/src/env.cc @@ -166,6 +166,26 @@ void Environment::PrintSyncTrace() const { fflush(stderr); } +void Environment::RunCleanup() { + while (!cleanup_hooks_.empty()) { + std::vector callbacks; + // Concatenate all vectors in cleanup_hooks_ + for (const auto& pair : cleanup_hooks_) + callbacks.insert(callbacks.end(), pair.second.begin(), pair.second.end()); + cleanup_hooks_.clear(); + std::sort(callbacks.begin(), callbacks.end(), + [](const CleanupHookCallback& a, const CleanupHookCallback& b) { + // Sort in descending order so that the last-inserted callbacks get run + // first. + return a.insertion_order_counter_ > b.insertion_order_counter_; + }); + + for (const CleanupHookCallback& cb : callbacks) { + cb.fun_(cb.arg_); + } + } +} + void Environment::RunAtExitCallbacks() { for (AtExitCallback at_exit : at_exit_functions_) { at_exit.cb_(at_exit.arg_); diff --git a/src/env.h b/src/env.h index 25db2e14e9..8da45476c6 100644 --- a/src/env.h +++ b/src/env.h @@ -609,6 +609,9 @@ class Environment { inline performance::performance_state* performance_state(); inline std::map* performance_marks(); + inline bool can_call_into_js() const; + inline void set_can_call_into_js(bool can_call_into_js); + inline void ThrowError(const char* errmsg); inline void ThrowTypeError(const char* errmsg); inline void ThrowRangeError(const char* errmsg); @@ -678,6 +681,10 @@ class Environment { bool RemovePromiseHook(promise_hook_func fn, void* arg); bool EmitNapiWarning(); + inline void AddCleanupHook(void (*fn)(void*), void* arg); + inline void RemoveCleanupHook(void (*fn)(void*), void* arg); + void RunCleanup(); + private: inline void ThrowError(v8::Local (*fun)(v8::Local), const char* errmsg); @@ -702,6 +709,8 @@ class Environment { size_t makecallback_cntr_; std::vector destroy_ids_list_; + bool can_call_into_js_ = true; + performance::performance_state* performance_state_ = nullptr; std::map performance_marks_; @@ -721,7 +730,7 @@ class Environment { char* http_parser_buffer_; http2::http2_state* http2_state_ = nullptr; - double* fs_stats_field_array_; + double* fs_stats_field_array_ = nullptr; struct AtExitCallback { void (*cb_)(void* arg); @@ -736,6 +745,15 @@ class Environment { }; std::vector promise_hooks_; + struct CleanupHookCallback { + void (*fun_)(void*); + void* arg_; + int64_t insertion_order_counter_; + }; + + std::unordered_map> cleanup_hooks_; + int64_t cleanup_hook_counter_ = 0; + static void EnvPromiseHook(v8::PromiseHookType type, v8::Local promise, v8::Local parent); diff --git a/src/node.cc b/src/node.cc index d6678dd967..71dd4575da 100644 --- a/src/node.cc +++ b/src/node.cc @@ -167,6 +167,9 @@ using v8::Value; using AsyncHooks = node::Environment::AsyncHooks; +static Mutex process_mutex; +static Mutex environ_mutex; + static bool print_eval = false; static bool force_repl = false; static bool syntax_check_only = false; @@ -1339,6 +1342,22 @@ void AddPromiseHook(v8::Isolate* isolate, promise_hook_func fn, void* arg) { env->AddPromiseHook(fn, arg); } +void AddEnvironmentCleanupHook(v8::Isolate* isolate, + void (*fun)(void* arg), + void* arg) { + Environment* env = Environment::GetCurrent(isolate); + env->AddCleanupHook(fun, arg); +} + + +void RemoveEnvironmentCleanupHook(v8::Isolate* isolate, + void (*fun)(void* arg), + void* arg) { + Environment* env = Environment::GetCurrent(isolate); + env->RemoveCleanupHook(fun, arg); +} + + CallbackScope::CallbackScope(Isolate* isolate, Local object, async_context asyncContext) @@ -1367,6 +1386,11 @@ InternalCallbackScope::InternalCallbackScope(Environment* env, 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(env->context(), env->isolate()->GetCurrentContext()); @@ -1414,6 +1438,7 @@ void InternalCallbackScope::Close() { Environment::TickInfo* tick_info = env_->tick_info(); + if (!env_->can_call_into_js()) return; if (tick_info->length() == 0) { env_->isolate()->RunMicrotasks(); } @@ -1433,6 +1458,8 @@ void InternalCallbackScope::Close() { CHECK_EQ(env_->current_async_id(), 0); CHECK_EQ(env_->trigger_id(), 0); + if (!env_->can_call_into_js()) return; + if (env_->tick_callback_function()->Call(process, 0, nullptr).IsEmpty()) { failed_ = true; } @@ -1792,6 +1819,7 @@ void AppendExceptionLine(Environment* env, if (!can_set_arrow || (mode == FATAL_ERROR && !err_obj->IsNativeError())) { if (env->printed_error()) return; + Mutex::ScopedLock lock(process_mutex); env->set_printed_error(true); uv_tty_reset_mode(); @@ -2955,6 +2983,7 @@ static void LinkedBinding(const FunctionCallbackInfo& args) { static void ProcessTitleGetter(Local property, const PropertyCallbackInfo& info) { + Mutex::ScopedLock lock(process_mutex); char buffer[512]; uv_get_process_title(buffer, sizeof(buffer)); info.GetReturnValue().Set(String::NewFromUtf8(info.GetIsolate(), buffer)); @@ -2964,6 +2993,7 @@ static void ProcessTitleGetter(Local property, static void ProcessTitleSetter(Local property, Local value, const PropertyCallbackInfo& info) { + Mutex::ScopedLock lock(process_mutex); node::Utf8Value title(info.GetIsolate(), value); // TODO(piscisaureus): protect with a lock uv_set_process_title(*title); @@ -2972,6 +3002,7 @@ static void ProcessTitleSetter(Local property, static void EnvGetter(Local property, const PropertyCallbackInfo& info) { + Mutex::ScopedLock lock(environ_mutex); Isolate* isolate = info.GetIsolate(); if (property->IsSymbol()) { return info.GetReturnValue().SetUndefined(); @@ -3004,6 +3035,7 @@ static void EnvGetter(Local property, static void EnvSetter(Local property, Local value, const PropertyCallbackInfo& info) { + Mutex::ScopedLock lock(environ_mutex); #ifdef __POSIX__ node::Utf8Value key(info.GetIsolate(), property); node::Utf8Value val(info.GetIsolate(), value); @@ -3024,6 +3056,7 @@ static void EnvSetter(Local property, static void EnvQuery(Local property, const PropertyCallbackInfo& info) { + Mutex::ScopedLock lock(environ_mutex); int32_t rc = -1; // Not found unless proven otherwise. if (property->IsString()) { #ifdef __POSIX__ @@ -3052,6 +3085,7 @@ static void EnvQuery(Local property, static void EnvDeleter(Local property, const PropertyCallbackInfo& info) { + Mutex::ScopedLock lock(environ_mutex); if (property->IsString()) { #ifdef __POSIX__ node::Utf8Value key(info.GetIsolate(), property); @@ -3070,6 +3104,7 @@ static void EnvDeleter(Local property, static void EnvEnumerator(const PropertyCallbackInfo& info) { + Mutex::ScopedLock lock(environ_mutex); Environment* env = Environment::GetCurrent(info); Isolate* isolate = env->isolate(); Local ctx = env->context(); @@ -3193,6 +3228,7 @@ static Local GetFeatures(Environment* env) { static void DebugPortGetter(Local property, const PropertyCallbackInfo& info) { + Mutex::ScopedLock lock(process_mutex); int port = debug_options.port(); #if HAVE_INSPECTOR if (port == 0) { @@ -3208,6 +3244,7 @@ static void DebugPortGetter(Local property, static void DebugPortSetter(Local property, Local value, const PropertyCallbackInfo& info) { + Mutex::ScopedLock lock(process_mutex); debug_options.set_port(value->Int32Value()); } @@ -4726,6 +4763,9 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data, env.set_trace_sync_io(false); const int exit_code = EmitExit(&env); + + env.set_can_call_into_js(false); + env.RunCleanup(); RunAtExit(&env); uv_key_delete(&thread_local_env); diff --git a/src/node.h b/src/node.h index 8d93fea7dc..368d0aa8a5 100644 --- a/src/node.h +++ b/src/node.h @@ -546,6 +546,19 @@ NODE_EXTERN void AddPromiseHook(v8::Isolate* isolate, promise_hook_func fn, void* arg); +/* This is a lot like node::AtExit, except that the hooks added via this + * function are run before the AtExit ones and will always be registered + * for the current Environment instance. + * These functions are safe to use in an addon supporting multiple + * threads/isolates. */ +NODE_EXTERN void AddEnvironmentCleanupHook(v8::Isolate* isolate, + void (*fun)(void* arg), + void* arg); + +NODE_EXTERN void RemoveEnvironmentCleanupHook(v8::Isolate* isolate, + void (*fun)(void* arg), + void* arg); + /* Returns the id of the current execution context. If the return value is * zero then no execution has been set. This will happen if the user handles * I/O from native code. */ diff --git a/src/node_api.cc b/src/node_api.cc index 8f8c400cdf..93c218bece 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -857,6 +857,28 @@ void napi_module_register(napi_module* mod) { node::node_module_register(nm); } +napi_status napi_add_env_cleanup_hook(napi_env env, + void (*fun)(void* arg), + void* arg) { + CHECK_ENV(env); + CHECK_ARG(env, fun); + + node::AddEnvironmentCleanupHook(env->isolate, fun, arg); + + return napi_ok; +} + +napi_status napi_remove_env_cleanup_hook(napi_env env, + void (*fun)(void* arg), + void* arg) { + CHECK_ENV(env); + CHECK_ARG(env, fun); + + node::RemoveEnvironmentCleanupHook(env->isolate, fun, arg); + + return napi_ok; +} + // Warning: Keep in-sync with napi_status enum const char* error_messages[] = {nullptr, "Invalid pointer passed as argument", diff --git a/src/node_api.h b/src/node_api.h index 82f2e1900e..5054a5df24 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -104,6 +104,13 @@ EXTERN_C_START NAPI_EXTERN void napi_module_register(napi_module* mod); +NAPI_EXTERN napi_status napi_add_env_cleanup_hook(napi_env env, + void (*fun)(void* arg), + void* arg); +NAPI_EXTERN napi_status napi_remove_env_cleanup_hook(napi_env env, + void (*fun)(void* arg), + void* arg); + NAPI_EXTERN napi_status napi_get_last_error_info(napi_env env, const napi_extended_error_info** result); diff --git a/src/node_contextify.cc b/src/node_contextify.cc index c73db420f1..c8830b45f3 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -986,6 +986,8 @@ class ContextifyScript : public BaseObject { const bool break_on_sigint, const FunctionCallbackInfo& args, TryCatch* try_catch) { + if (!env->can_call_into_js()) + return false; if (!ContextifyScript::InstanceOf(env, args.Holder())) { env->ThrowTypeError( "Script methods can only be called on script instances."); diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 452e085a2d..0482bce527 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -736,6 +736,9 @@ static int X509_up_ref(X509* cert) { static X509_STORE* NewRootCertStore() { static std::vector root_certs_vector; + static Mutex root_certs_vector_mutex; + Mutex::ScopedLock lock(root_certs_vector_mutex); + if (root_certs_vector.empty()) { for (size_t i = 0; i < arraysize(root_certs); i++) { BIO* bp = NodeBIO::NewFixed(root_certs[i], strlen(root_certs[i])); diff --git a/src/node_http2.cc b/src/node_http2.cc index c1e178f717..6f726a9339 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -28,7 +28,7 @@ Freelist header_free_list; Freelist data_chunks_free_list; -Nghttp2Session::Callbacks Nghttp2Session::callback_struct_saved[2] = { +const Nghttp2Session::Callbacks Nghttp2Session::callback_struct_saved[2] = { Callbacks(false), Callbacks(true)}; diff --git a/src/node_http2_core.h b/src/node_http2_core.h index 69b9891d58..8def84cc18 100644 --- a/src/node_http2_core.h +++ b/src/node_http2_core.h @@ -279,7 +279,7 @@ class Nghttp2Session { }; /* Use callback_struct_saved[kHasGetPaddingCallback ? 1 : 0] */ - static Callbacks callback_struct_saved[2]; + static const Callbacks callback_struct_saved[2]; nghttp2_session* session_; uv_loop_t* loop_; diff --git a/src/string_search.cc b/src/string_search.cc deleted file mode 100644 index 326fba7c4a..0000000000 --- a/src/string_search.cc +++ /dev/null @@ -1,11 +0,0 @@ -#include "string_search.h" - -namespace node { -namespace stringsearch { - -int StringSearchBase::kBadCharShiftTable[kUC16AlphabetSize]; -int StringSearchBase::kGoodSuffixShiftTable[kBMMaxShift + 1]; -int StringSearchBase::kSuffixTable[kBMMaxShift + 1]; - -} // namespace stringsearch -} // namespace node diff --git a/src/string_search.h b/src/string_search.h index 73e90f5873..f2fed07e2e 100644 --- a/src/string_search.h +++ b/src/string_search.h @@ -85,12 +85,12 @@ class StringSearchBase { static const int kBMMinPatternLength = 8; // Store for the BoyerMoore(Horspool) bad char shift table. - static int kBadCharShiftTable[kUC16AlphabetSize]; + int kBadCharShiftTable[kUC16AlphabetSize]; // Store for the BoyerMoore good suffix shift table. - static int kGoodSuffixShiftTable[kBMMaxShift + 1]; + int kGoodSuffixShiftTable[kBMMaxShift + 1]; // Table used temporarily while building the BoyerMoore good suffix // shift table. - static int kSuffixTable[kBMMaxShift + 1]; + int kSuffixTable[kBMMaxShift + 1]; }; template