Implement timeout for callbacks

sass · Aug 29, 2015 · f0fca38 · f0fca38
1 parent 660cc66
commit f0fca38
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 23 deletions.
diff --git a/src/binding.cpp b/src/binding.cpp
@@ -18,7 +18,15 @@ Sass_Import_List sass_importer(const char* cur_path, Sass_Importer_Entry cb, str
   argv.push_back((void*)prev_path);
 
   TRACEINST(&bridge) << "Importer will be executed";
-  return bridge(argv);
+
+  Sass_Import_List retval = bridge(argv);
+  if (bridge.timedout) {
+    TRACEINST(&bridge) << "Importer timeout";
+    retval = sass_make_import_list(1);
+    retval[0] = sass_make_import_entry(0, 0, 0);
+    sass_import_set_error(retval[0], "Importer timed out or blocked (>2000ms)", -1, -1);
+  }
+  return retval; 
 }
 
 union Sass_Value* sass_custom_function(const union Sass_Value* s_args, Sass_Function_Entry cb, struct Sass_Options* opts)
@@ -33,7 +41,12 @@ union Sass_Value* sass_custom_function(const union Sass_Value* s_args, Sass_Func
 
   try {
     TRACEINST(&bridge) << "Function will be executed";
-    return bridge(argv);
+    Sass_Value *retval = bridge(argv);
+    if (bridge.timedout) {
+      TRACEINST(&bridge) << "Function timeout";
+      return sass_make_error("Function timed out or blocked (>2000ms)");
+    } else
+      return retval;
   }
   catch (const std::exception& e) {
     return sass_make_error(e.what());

diff --git a/src/callback_bridge.h b/src/callback_bridge.h
@@ -20,6 +20,7 @@ class CallbackBridge {
 
     // Executes the callback
     T operator()(std::vector<void*>);
+    int timedout;
 
   protected:
     // We will expose a bridge object to the JS callback that wraps this instance so we don't loose context.
@@ -60,7 +61,7 @@ template <typename T, typename L>
 Nan::Persistent<v8::Function> CallbackBridge<T, L>::wrapper_constructor;
 
 template <typename T, typename L>
-CallbackBridge<T, L>::CallbackBridge(v8::Local<v8::Function> callback, bool is_sync) : callback(new Nan::Callback(callback)), is_sync(is_sync) {
+CallbackBridge<T, L>::CallbackBridge(v8::Local<v8::Function> callback, bool is_sync) : timedout(0), callback(new Nan::Callback(callback)), is_sync(is_sync) {
   /* 
    * This is invoked from the main JavaScript thread.
    * V8 context is available.
@@ -70,7 +71,7 @@ CallbackBridge<T, L>::CallbackBridge(v8::Local<v8::Function> callback, bool is_s
 }
 
 template <typename T, typename L>
-CallbackBridge<T, L>::CallbackBridge(const CallbackBridge<T,L>& other) : callback(new Nan::Callback(other.callback->GetFunction())), is_sync(other.is_sync) {
+CallbackBridge<T, L>::CallbackBridge(const CallbackBridge<T,L>& other) : timedout(0), callback(new Nan::Callback(other.callback->GetFunction())), is_sync(other.is_sync) {
   /* 
    * This is invoked from the main JavaScript thread.
    * V8 context is available.
@@ -99,6 +100,7 @@ CallbackBridge<T, L>::operator= (const CallbackBridge<T,L>& other)
 
      this->callback = new Nan::Callback(other.callback->GetFunction());
      this->is_sync = other.is_sync;
+     this->timedout = 0;
      init_uv();
      init_wrapper();
    }
@@ -174,12 +176,12 @@ T CallbackBridge<T, L>::operator()(std::vector<void*> argv) {
      *     async I/O executed from JavaScript callbacks.
      */
     this->argv = argv;
-
+    this->timedout = 0;
     uv_mutex_lock(&this->cv_mutex);
     this->has_returned = false;
     uv_async_send(this->async);
-    while (!this->has_returned) {
-      uv_cond_wait(&this->condition_variable, &this->cv_mutex);
+    while (!this->has_returned && this->timedout == 0) {
+      this->timedout = uv_cond_timedwait(&this->condition_variable, &this->cv_mutex, 2 * (uint64_t)1e9);
     }
     uv_mutex_unlock(&this->cv_mutex);
     return this->return_value;
@@ -199,16 +201,18 @@ void CallbackBridge<T, L>::dispatched_async_uv_callback(uv_async_t *req) {
    * from types invoked by pre_process_args() and
    * post_process_args().
    */
-  Nan::HandleScope scope;
-  Nan::TryCatch try_catch;
+  if (!bridge->timedout) {
+    Nan::HandleScope scope;
+    Nan::TryCatch try_catch;
 
-  std::vector<v8::Local<v8::Value>> argv_v8 = bridge->pre_process_args(bridge->argv);
-  argv_v8.push_back(Nan::New(bridge->wrapper));
+    std::vector<v8::Local<v8::Value>> argv_v8 = bridge->pre_process_args(bridge->argv);
+    argv_v8.push_back(Nan::New(bridge->wrapper));
 
-  bridge->callback->Call(argv_v8.size(), &argv_v8[0]);
+    bridge->callback->Call(argv_v8.size(), &argv_v8[0]);
 
-  if (try_catch.HasCaught()) {
-    Nan::FatalException(try_catch);
+    if (try_catch.HasCaught()) {
+      Nan::FatalException(try_catch);
+    }
   }
 }
 
@@ -225,18 +229,20 @@ NAN_METHOD(CallbackBridge<T COMMA L>::ReturnCallback) {
   CallbackBridge<T, L>* bridge = static_cast<CallbackBridge<T, L>*>(Nan::GetInternalFieldPointer(info.This(), 0));
   Nan::TryCatch try_catch;
 
-  bridge->return_value = bridge->post_process_return_value(info[0]);
+  if (!bridge->timedout) {
+    bridge->return_value = bridge->post_process_return_value(info[0]);
 
-  {
-    uv_mutex_lock(&bridge->cv_mutex);
-    bridge->has_returned = true;
-    uv_mutex_unlock(&bridge->cv_mutex);
-  }
+    {
+      uv_mutex_lock(&bridge->cv_mutex);
+      bridge->has_returned = true;
+      uv_mutex_unlock(&bridge->cv_mutex);
+    }
 
-  uv_cond_broadcast(&bridge->condition_variable);
+    uv_cond_broadcast(&bridge->condition_variable);
 
-  if (try_catch.HasCaught()) {
-    Nan::FatalException(try_catch);
+    if (try_catch.HasCaught()) {
+      Nan::FatalException(try_catch);
+    }
   }
 }