Improving the implementation of DisallowJavascriptExecutionScope.

Author: iamstolis

graal-nodejs/deps/v8/src/graal/graal_function.cc

@@ -92,7 +92,7 @@ v8::Local<v8::Value> GraalFunction::Call(v8::Local<v8::Value> recv, int argc, v8
         reinterpret_cast<GraalIsolate*> (GraalIsolate::GetCurrent())->ReportAPIFailure(
                 "v8::Function::Call", "Function to be called is a null pointer");
     }
-    if (!Isolate()->GetJSExecutionAllowed()) {
+    if (!Isolate()->CheckJSExecutionAllowed()) {
         return nullptr;
     }
     jobject java_receiver = reinterpret_cast<GraalValue*> (*recv)->GetJavaObject();

graal-nodejs/deps/v8/src/graal/graal_isolate.cc

@@ -1518,3 +1518,11 @@ void GraalIsolate::RequestInterrupt(v8::InterruptCallback callback, void* data)
         jvm_->DetachCurrentThread();
     }
 }
+
+void GraalIsolate::JSExecutionViolation(JSExecutionAction action) {
+    if (action == kJSExecutionThrow) {
+        ThrowException(v8::String::NewFromUtf8(reinterpret_cast<v8::Isolate*> (this), "Illegal operation.").ToLocalChecked());
+    } else {
+        ReportAPIFailure("DisallowJavascriptExecutionScope", "Illegal operation.");
+    }
+}

graal-nodejs/deps/v8/src/graal/graal_isolate.h

@@ -705,16 +705,29 @@ class GraalIsolate {
         return &microtask_queue_;
     }
 
-    inline bool SetJSExecutionAllowed(bool allowed) {
-        bool old = js_execution_allowed_;
-        js_execution_allowed_ = allowed;
+    enum JSExecutionAction {
+        kJSExecutionAllowed,
+        kJSExecutionThrow,
+        kJSExecutionCrash,
+    };
+
+    inline JSExecutionAction SetJSExecutionAction(JSExecutionAction action) {
+        JSExecutionAction old = js_execution_action_;
+        js_execution_action_ = action;
         return old;
     }
 
-    inline bool GetJSExecutionAllowed() {
-        return js_execution_allowed_;
+    inline bool CheckJSExecutionAllowed() {
+        if (js_execution_action_ == kJSExecutionAllowed) {
+            return true;
+        } else {
+            JSExecutionViolation(js_execution_action_);
+            return false;
+        }
     }
 
+    void JSExecutionViolation(JSExecutionAction action);
+
     static void SetFlags(int argc, char** argv) {
         char** old_argv = GraalIsolate::argv;
         int old_argc = GraalIsolate::argc;
@@ -795,7 +808,7 @@ class GraalIsolate {
     intptr_t stack_bottom_;
     size_t stack_size_limit_;
     bool main_;
-    bool js_execution_allowed_ = true;
+    JSExecutionAction js_execution_action_ = kJSExecutionAllowed;
     double return_value_;
     static bool abort_on_uncaught_exception_;
     static bool internal_error_check_;

graal-nodejs/deps/v8/src/graal/graal_object.cc

@@ -70,9 +70,13 @@ v8::Local<v8::Object> GraalObject::New(v8::Isolate* isolate) {
 }
 
 bool GraalObject::Set(v8::Local<v8::Value> key, v8::Local<v8::Value> value) {
+    GraalIsolate* graal_isolate = Isolate();
+    if (!graal_isolate->CheckJSExecutionAllowed()) {
+        return false;
+    }
     jobject java_key = reinterpret_cast<GraalValue*> (*key)->GetJavaObject();
     jobject java_value = reinterpret_cast<GraalValue*> (*value)->GetJavaObject();
-    JNI_CALL(bool, success, Isolate(), GraalAccessMethod::object_set, Boolean, GetJavaObject(), java_key, java_value);
+    JNI_CALL(bool, success, graal_isolate, GraalAccessMethod::object_set, Boolean, GetJavaObject(), java_key, java_value);
     return success;
 }
 

graal-nodejs/deps/v8/src/graal/v8.cc

@@ -2885,20 +2885,27 @@ namespace v8 {
 
     Isolate::DisallowJavascriptExecutionScope::DisallowJavascriptExecutionScope(Isolate* isolate, OnFailure on_failure) {
         isolate_ = isolate;
-        was_execution_allowed_throws_ = reinterpret_cast<GraalIsolate*> (isolate)->SetJSExecutionAllowed(false);
+        GraalIsolate::JSExecutionAction newAction = (on_failure == THROW_ON_FAILURE) ? GraalIsolate::kJSExecutionThrow : GraalIsolate::kJSExecutionCrash;
+        GraalIsolate::JSExecutionAction oldAction = reinterpret_cast<GraalIsolate*> (isolate)->SetJSExecutionAction(newAction);
+        was_execution_allowed_throws_ = (oldAction == GraalIsolate::kJSExecutionThrow);
+        was_execution_allowed_assert_ = (oldAction == GraalIsolate::kJSExecutionCrash);
     }
 
     Isolate::DisallowJavascriptExecutionScope::~DisallowJavascriptExecutionScope() {
-        reinterpret_cast<GraalIsolate*> (isolate_)->SetJSExecutionAllowed(was_execution_allowed_throws_);
+        GraalIsolate::JSExecutionAction action = was_execution_allowed_throws_ ? GraalIsolate::kJSExecutionThrow : (was_execution_allowed_assert_ ? GraalIsolate::kJSExecutionCrash : GraalIsolate::kJSExecutionAllowed);
+        reinterpret_cast<GraalIsolate*> (isolate_)->SetJSExecutionAction(action);
     }
 
     Isolate::AllowJavascriptExecutionScope::AllowJavascriptExecutionScope(Isolate* isolate) {
         isolate_ = isolate;
-        was_execution_allowed_throws_ = reinterpret_cast<GraalIsolate*> (isolate)->SetJSExecutionAllowed(true);
+        GraalIsolate::JSExecutionAction oldAction = reinterpret_cast<GraalIsolate*> (isolate)->SetJSExecutionAction(GraalIsolate::kJSExecutionAllowed);
+        was_execution_allowed_throws_ = (oldAction == GraalIsolate::kJSExecutionThrow);
+        was_execution_allowed_assert_ = (oldAction == GraalIsolate::kJSExecutionCrash);
     }
 
     Isolate::AllowJavascriptExecutionScope::~AllowJavascriptExecutionScope() {
-        reinterpret_cast<GraalIsolate*> (isolate_)->SetJSExecutionAllowed(was_execution_allowed_throws_);
+        GraalIsolate::JSExecutionAction action = was_execution_allowed_throws_ ? GraalIsolate::kJSExecutionThrow : (was_execution_allowed_assert_ ? GraalIsolate::kJSExecutionCrash : GraalIsolate::kJSExecutionAllowed);
+        reinterpret_cast<GraalIsolate*> (isolate_)->SetJSExecutionAction(action);
     }
 
     void HeapProfiler::RemoveBuildEmbedderGraphCallback(BuildEmbedderGraphCallback callback, void* data) {