Skip to content

Commit

Permalink
core: implement runtime flag to print warn on sync
Browse files Browse the repository at this point in the history
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 at a specific location in the past and so will
print the warning every time.

This does not print the warning for the first synchronous execution of
the script or during the process 'exit' event. Allowing all the
require(), etc. statements to run that are necessary to prepare the
environment.
  • Loading branch information
trevnorris committed May 15, 2015
1 parent 4e2f999 commit c6ea43d
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 1 deletion.
39 changes: 39 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -325,6 +326,44 @@ 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_name_s(isolate_, sf->GetFunctionName());
node::Utf8Value sn_s(isolate_, sf->GetScriptName());
int ln = sf->GetLineNumber();
int cn = sf->GetColumn();

if (sf->IsEval()) {
if (sf->GetScriptId() == v8::Message::kNoScriptIdInfo)
fprintf(stderr, " at [eval]:%i:%i\n", ln, cn);
else
fprintf(stderr, " at [eval] (%s:%i:%i)\n", *sn_s, ln, cn);
return;
}

if (strcmp(*fn_name_s, "") == 0) {
fprintf(stderr, " at %s:%i:%i\n", *sn_s, ln, cn);
} else {
fprintf(stderr, " at %s (%s:%i:%i)\n", *fn_name_s, *sn_s, ln, cn);
}
}
}

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);
}
Expand Down
4 changes: 4 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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_;
Expand Down
18 changes: 18 additions & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -2834,6 +2837,13 @@ 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()));
// Don't env->set_warn_on_sync(true) because it will be enabled after
// LoadEnvironment() has run.
}

size_t exec_path_len = 2 * PATH_MAX;
char* exec_path = new char[exec_path_len];
Local<String> exec_path_value;
Expand Down Expand Up @@ -3061,6 +3071,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"
Expand Down Expand Up @@ -3180,6 +3192,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 ||
Expand Down Expand Up @@ -3887,6 +3901,8 @@ static void StartNodeInstance(void* arg) {

LoadEnvironment(env);

env->set_warn_on_sync(warn_on_sync);

// Enable debugger
if (instance_data->use_debug_agent())
EnableDebug(env);
Expand All @@ -3911,6 +3927,8 @@ static void StartNodeInstance(void* arg) {
} while (more == true);
}

env->set_warn_on_sync(false);

int exit_code = EmitExit(env);
if (instance_data->is_main())
instance_data->set_exit_code(exit_code);
Expand Down
2 changes: 2 additions & 0 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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__, \
Expand Down
1 change: 1 addition & 0 deletions src/node_zlib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
4 changes: 3 additions & 1 deletion src/spawn_sync.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit c6ea43d

Please sign in to comment.