From 80926f2c6101b1dcd86bafa75578453d2c5f5fbb Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Thu, 14 May 2015 11:46:54 -0600 Subject: [PATCH] core: implement runtime flag to pring warn on sync Use the --warn-on-sync flag to print a stack trace whenever a sync method is used. (e.g. fs.readFileSync()) It does not track if the warning has occurred as a specific location in the past and so will print the warning every time the function is used. This does not print the warning for the first synchronous execution of the script. This will allow all the require(), etc. statements to run that are necessary to prepare the environment. --- src/env-inl.h | 24 ++++++++++++++++++++++++ src/env.h | 4 ++++ src/node.cc | 21 +++++++++++++++++++++ src/node.h | 1 + src/node.js | 3 +++ src/node_crypto.cc | 2 ++ src/node_file.cc | 1 + src/node_zlib.cc | 1 + src/spawn_sync.cc | 4 +++- 9 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/env-inl.h b/src/env-inl.h index a0815acaccb926..53015e9d15d7db 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -176,6 +176,7 @@ inline Environment::Environment(v8::Local context, using_abort_on_uncaught_exc_(false), using_asyncwrap_(false), printed_error_(false), + warn_on_sync_(false), debugger_agent_(this), context_(context->GetIsolate(), context) { // We'll be creating new objects so make sure we've entered the context. @@ -325,6 +326,29 @@ inline void Environment::set_printed_error(bool value) { printed_error_ = value; } +inline void Environment::print_sync_warn() const { + if (!warn_on_sync_) + return; + + v8::HandleScope handle_scope(isolate_); + v8::Local stack = v8::StackTrace::CurrentStackTrace( + isolate_, 10, v8::StackTrace::kDetailed); + + fprintf(stderr, "WARNING: Detected use of sync API\n"); + for (int i = 0; i < stack->GetFrameCount() - 1; i++) { + v8::Local sf = stack->GetFrame(i); + node::Utf8Value n(isolate_, sf->GetFunctionName()); + node::Utf8Value m(isolate_, sf->GetScriptName()); + int ln = sf->GetLineNumber(); + int cl = sf->GetColumn(); + fprintf(stderr, " at %s (%s:%i:%i)\n", *n, *m, ln, cl); + } +} + +inline void Environment::set_warn_on_sync(bool value) { + warn_on_sync_ = value; +} + inline Environment* Environment::from_cares_timer_handle(uv_timer_t* handle) { return ContainerOf(&Environment::cares_timer_handle_, handle); } diff --git a/src/env.h b/src/env.h index e327786e36b907..a02ddec3936de5 100644 --- a/src/env.h +++ b/src/env.h @@ -420,6 +420,9 @@ class Environment { inline bool printed_error() const; inline void set_printed_error(bool value); + inline void print_sync_warn() const; + inline void set_warn_on_sync(bool value); + inline void ThrowError(const char* errmsg); inline void ThrowTypeError(const char* errmsg); inline void ThrowRangeError(const char* errmsg); @@ -506,6 +509,7 @@ class Environment { bool using_abort_on_uncaught_exc_; bool using_asyncwrap_; bool printed_error_; + bool warn_on_sync_; debugger::Agent debugger_agent_; HandleWrapQueue handle_wrap_queue_; diff --git a/src/node.cc b/src/node.cc index 4cafe96f445dd1..02aa4cf289eb67 100644 --- a/src/node.cc +++ b/src/node.cc @@ -102,6 +102,8 @@ using v8::Promise; using v8::PromiseRejectMessage; using v8::PropertyCallbackInfo; using v8::SealHandleScope; +using v8::StackFrame; +using v8::StackTrace; using v8::String; using v8::TryCatch; using v8::Uint32; @@ -114,6 +116,7 @@ static bool force_repl = false; static bool trace_deprecation = false; static bool throw_deprecation = false; static bool abort_on_uncaught_exception = false; +static bool warn_on_sync = false; static const char* eval_string = nullptr; static unsigned int preload_module_count = 0; static const char** preload_modules = nullptr; @@ -1019,6 +1022,14 @@ void SetupPromises(const FunctionCallbackInfo& args) { } +void EnableSyncWarn(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + env->set_warn_on_sync(true); + env->process_object()->Delete( + FIXED_ONE_BYTE_STRING(args.GetIsolate(), "_enableSyncWarn")); +} + + Handle MakeCallback(Environment* env, Handle recv, const Handle callback, @@ -2834,6 +2845,14 @@ void SetupProcessObject(Environment* env, READONLY_PROPERTY(process, "traceDeprecation", True(env->isolate())); } + // --warn-on-sync + if (warn_on_sync) { + READONLY_PROPERTY(process, "warnOnSync", True(env->isolate())); + // Only add this method if the flag has been enabled. + env->SetMethod(process, "_enableSyncWarn", EnableSyncWarn); + // Don't env->set_warn_on_sync(true) because it will be enabled from JS. + } + size_t exec_path_len = 2 * PATH_MAX; char* exec_path = new char[exec_path_len]; Local exec_path_value; @@ -3180,6 +3199,8 @@ static void ParseArgs(int* argc, no_deprecation = true; } else if (strcmp(arg, "--trace-deprecation") == 0) { trace_deprecation = true; + } else if (strcmp(arg, "--warn-on-sync") == 0) { + warn_on_sync = true; } else if (strcmp(arg, "--throw-deprecation") == 0) { throw_deprecation = true; } else if (strcmp(arg, "--abort-on-uncaught-exception") == 0 || diff --git a/src/node.h b/src/node.h index acdfe5740740ef..efe065857d8dd1 100644 --- a/src/node.h +++ b/src/node.h @@ -335,6 +335,7 @@ NODE_DEPRECATED("Use DecodeWrite(isolate, ...)", return DecodeWrite(v8::Isolate::GetCurrent(), buf, buflen, val, encoding); }) + #ifdef _WIN32 NODE_EXTERN v8::Local WinapiErrnoException( v8::Isolate* isolate, diff --git a/src/node.js b/src/node.js index 55b1d782ce82b0..0e4c6b8a9b83b8 100644 --- a/src/node.js +++ b/src/node.js @@ -45,6 +45,9 @@ process.argv[0] = process.execPath; + if (process.warnOnSync) + process.nextTick(process._enableSyncWarn); + // There are various modes that Node can run in. The most common two // are running from a script and running the REPL - but there are a few // others like the debugger or running --eval arguments. Here we decide diff --git a/src/node_crypto.cc b/src/node_crypto.cc index e49545810d3f78..cd53add395f311 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -4630,6 +4630,7 @@ void PBKDF2(const FunctionCallbackInfo& args) { EIO_PBKDF2, EIO_PBKDF2After); } else { + env->print_sync_warn(); Local argv[2]; EIO_PBKDF2(req); EIO_PBKDF2After(req, argv); @@ -4786,6 +4787,7 @@ void RandomBytes(const FunctionCallbackInfo& args) { RandomBytesAfter); args.GetReturnValue().Set(obj); } else { + env->print_sync_warn(); Local argv[2]; RandomBytesWork(req->work_req()); RandomBytesCheck(req, argv); diff --git a/src/node_file.cc b/src/node_file.cc index 095710ef37e32b..90508c4eca7610 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -280,6 +280,7 @@ struct fs_req_wrap { #define SYNC_DEST_CALL(func, path, dest, ...) \ fs_req_wrap req_wrap; \ + env->print_sync_warn(); \ int err = uv_fs_ ## func(env->event_loop(), \ &req_wrap.req, \ __VA_ARGS__, \ diff --git a/src/node_zlib.cc b/src/node_zlib.cc index 884c244f9f0afc..9f91070c732acb 100644 --- a/src/node_zlib.cc +++ b/src/node_zlib.cc @@ -180,6 +180,7 @@ class ZCtx : public AsyncWrap { ctx->chunk_size_ = out_len; if (!async) { + ctx->env()->print_sync_warn(); // sync version Process(work_req); if (CheckError(ctx)) diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index 15e4fe8b103b0d..ca36474c87d291 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -349,7 +349,9 @@ void SyncProcessRunner::Initialize(Handle target, void SyncProcessRunner::Spawn(const FunctionCallbackInfo& args) { - SyncProcessRunner p(Environment::GetCurrent(args)); + Environment* env = Environment::GetCurrent(args); + env->print_sync_warn(); + SyncProcessRunner p(env); Local result = p.Run(args[0]); args.GetReturnValue().Set(result); }