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.

Author: trevnorris

src/env-inl.h

@@ -176,6 +176,7 @@ inline Environment::Environment(v8::Local<v8::Context> 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,34 @@ 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<v8::StackTrace> 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<v8::StackFrame> sf = stack->GetFrame(i);
+    node::Utf8Value fn_s(isolate_, sf->GetFunctionName());
+    const char* fn;
+    if (strcmp(*fn_s, "") == 0)
+      fn = "(anonymous)";
+    else
+      fn = *fn_s;
+    node::Utf8Value m(isolate_, sf->GetScriptName());
+    int ln = sf->GetLineNumber();
+    int cl = sf->GetColumn();
+    fprintf(stderr, "    at %s (%s:%i:%i)\n", fn, *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);
 }

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_;

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<Value>& args) {
 }
 
 
+void EnableSyncWarn(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  env->set_warn_on_sync(true);
+  env->process_object()->Delete(
+      FIXED_ONE_BYTE_STRING(args.GetIsolate(), "_enableSyncWarn"));
+}
+
+
 Handle<Value> MakeCallback(Environment* env,
                            Handle<Value> recv,
                            const Handle<Function> 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<String> exec_path_value;
@@ -3061,6 +3080,8 @@ static void PrintHelp() {
          "function is used\n"
          "  --trace-deprecation  show stack traces on deprecations\n"
          "  --v8-options         print v8 command line options\n"
+         "  --warn-on-sync       print warning message when sync\n"
+         "                       function is used\n"
 #if defined(NODE_HAVE_I18N_SUPPORT)
          "  --icu-data-dir=dir   set ICU data load path to dir\n"
          "                         (overrides NODE_ICU_DATA)\n"
@@ -3180,6 +3201,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 ||

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<v8::Value> WinapiErrnoException(
     v8::Isolate* isolate,

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

src/node_crypto.cc

@@ -4630,6 +4630,7 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
                   EIO_PBKDF2,
                   EIO_PBKDF2After);
   } else {
+    env->print_sync_warn();
     Local<Value> argv[2];
     EIO_PBKDF2(req);
     EIO_PBKDF2After(req, argv);
@@ -4786,6 +4787,7 @@ void RandomBytes(const FunctionCallbackInfo<Value>& args) {
                   RandomBytesAfter);
     args.GetReturnValue().Set(obj);
   } else {
+    env->print_sync_warn();
     Local<Value> argv[2];
     RandomBytesWork(req->work_req());
     RandomBytesCheck(req, argv);

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__,                                         \

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))

src/spawn_sync.cc

@@ -349,7 +349,9 @@ void SyncProcessRunner::Initialize(Handle<Object> target,
 
 
 void SyncProcessRunner::Spawn(const FunctionCallbackInfo<Value>& args) {
-  SyncProcessRunner p(Environment::GetCurrent(args));
+  Environment* env = Environment::GetCurrent(args);
+  env->print_sync_warn();
+  SyncProcessRunner p(env);
   Local<Value> result = p.Run(args[0]);
   args.GetReturnValue().Set(result);
 }