[GR-27881] Backport: Improving the handling of termination exceptions.

PullRequest: js/1798

Author: gilles-duboscq

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

@@ -1243,8 +1243,9 @@ void GraalIsolate::CancelTerminateExecution() {
         jvm_->AttachCurrentThread((void**) &env, nullptr);
     } else {
         env = current_isolate->GetJNIEnv();
-        // The following line breaks test-vm-timeout-rethrow
-        // env->ExceptionClear(); // Clear potential KillException
+        if (current_isolate == this) {
+            env->ExceptionClear(); // Clear potential termination exception in this thread
+        }
     }
     jmethodID method_id = GetJNIMethod(GraalAccessMethod::isolate_cancel_terminate_execution);
     env->functions->CallVoidMethod(env, access_, method_id);

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

@@ -937,9 +937,9 @@ namespace v8 {
     TryCatch::~TryCatch() {
         GraalIsolate* graal_isolate = reinterpret_cast<GraalIsolate*> (isolate_);
         graal_isolate->TryCatchExit();
-        if (!rethrow_ && HasCaught()) {
+        if (!rethrow_ && HasCaught() && !HasTerminated()) {
             JNIEnv* env = graal_isolate->GetJNIEnv();
-            if (is_verbose_ && !HasTerminated()) {
+            if (is_verbose_) {
                 jthrowable java_exception = env->ExceptionOccurred();
                 Local<Value> exception = Exception();
                 Local<v8::Message> message = Message();

graal-nodejs/test/graal/unit/spawn.js

@@ -95,7 +95,7 @@ describe('Spawn', function () {
         it('should print help when ' + option + ' option is used', function () {
             var result = spawnSync(process.execPath, [option]);
             assert.strictEqual(result.status, 0);
-            assert.match(result.stdout.toString(), /^\n?Usage:/);
+            assert.match(result.stdout.toString(), /Options:/);
         });
     });
     if (typeof java === 'object') {

graal-nodejs/test/graal/unit/try_catch.cc

@@ -130,8 +130,42 @@ EXPORT_TO_JS(HasCaughtNestedBoth) {
 
 EXPORT_TO_JS(HasTerminatedNoException) {
     TryCatch tryCatch(args.GetIsolate());
-    //bool result = tryCatch.HasTerminated(); //TODO
-    bool result = false;
+    bool result = tryCatch.HasTerminated();
+    args.GetReturnValue().Set(result);
+}
+
+EXPORT_TO_JS(HasTerminatedNestedOuter) {
+    Isolate* isolate = args.GetIsolate();
+    bool result = true;
+    TryCatch tryCatch(isolate);
+    result &= !tryCatch.HasTerminated();
+    TryCatch_InvokeCallback(args);
+    result &= tryCatch.HasTerminated();
+    isolate->CancelTerminateExecution();
+    result &= !tryCatch.HasTerminated();
+    args.GetReturnValue().Set(result);
+}
+
+EXPORT_TO_JS(HasTerminatedNestedInner) {
+    Isolate* isolate = args.GetIsolate();
+    // The following TryCatch looks unused but we are testing
+    // that it does not swallow the termination exception
+    TryCatch tryCatch(isolate);
+    isolate->TerminateExecution();
+    TryCatch_InvokeCallback(args);
+}
+
+EXPORT_TO_JS(HasTerminatedBasic) {
+    Isolate* isolate = args.GetIsolate();
+    bool result = true;
+    TryCatch tryCatch(isolate);
+    result &= !tryCatch.HasTerminated();
+    isolate->TerminateExecution();
+    result &= !tryCatch.HasTerminated();
+    TryCatch_InvokeCallback(args);
+    result &= tryCatch.HasTerminated();
+    isolate->CancelTerminateExecution();
+    result &= !tryCatch.HasTerminated();
     args.GetReturnValue().Set(result);
 }
 

graal-nodejs/test/graal/unit/try_catch.js

@@ -121,9 +121,26 @@ describe('TryCatch', function () {
         });
     });
     describe('HasTerminated', function () {
-        it('should be callable', function () {
+        it('should return false when the execution is not terminated', function () {
             assert.strictEqual(module.TryCatch_HasTerminatedNoException(), false);
         });
+        it('should return true when the execution is terminated', function () {
+            assert.strictEqual(module.TryCatch_HasTerminatedBasic(function() {
+                while (true); // This loop should be terminated
+            }), true);
+        });
+        it('should return true when the termination was not cancelled by innner TryCatch', function () {
+            var skipped = true;
+            assert.strictEqual(module.TryCatch_HasTerminatedNestedOuter(function() {
+                module.TryCatch_HasTerminatedNestedInner(function() {
+                    while (true); // This loop should be terminated
+                });
+                // This code should be skipped
+                skipped = false;
+                assert.fail();
+            }), true);
+            assert.ok(skipped);
+        });
     });
     describe('Fatal Error', function () {
         it('should not be triggered when an error is thrown from a promise job (part 1)', function (done) {