diff --git a/test-app/app/src/main/assets/app/mainpage.js b/test-app/app/src/main/assets/app/mainpage.js index 2ff19f79d..863f6c0e4 100644 --- a/test-app/app/src/main/assets/app/mainpage.js +++ b/test-app/app/src/main/assets/app/mainpage.js @@ -20,6 +20,7 @@ shared.runRuntimeTests(); shared.runWorkerTests(); require("./tests/testWebAssembly"); require("./tests/testMultithreadedJavascript"); +require("./tests/testWorkerTerminateDuringLoad"); require("./tests/testInterfaceDefaultMethods"); require("./tests/testInterfaceStaticMethods"); require("./tests/testMetadata"); diff --git a/test-app/app/src/main/assets/app/tests/testWorkerTerminateDuringLoad.js b/test-app/app/src/main/assets/app/tests/testWorkerTerminateDuringLoad.js new file mode 100644 index 000000000..29ede32be --- /dev/null +++ b/test-app/app/src/main/assets/app/tests/testWorkerTerminateDuringLoad.js @@ -0,0 +1,33 @@ +describe("Worker terminate during module load", function () { + var ITERATIONS = 3; + // Long enough to outlast the worker's isolate setup, short enough to keep + // the spec well inside the jasmine timeout. + var TERMINATE_AFTER = 150; + var SETTLE_AFTER = 250; + + it("should not report an error or crash when terminate() interrupts the worker's module body", function (done) { + var errors = []; + + function iteration(remaining) { + if (remaining === 0) { + expect(errors).toEqual([]); + done(); + return; + } + + var worker = new Worker("./workerTerminateDuringLoadWorker.js"); + worker.onerror = function (e) { + errors.push(e.message); + }; + + setTimeout(function () { + worker.terminate(); + setTimeout(function () { + iteration(remaining - 1); + }, SETTLE_AFTER); + }, TERMINATE_AFTER); + } + + iteration(ITERATIONS); + }); +}); diff --git a/test-app/app/src/main/assets/app/tests/workerTerminateDuringLoadWorker.js b/test-app/app/src/main/assets/app/tests/workerTerminateDuringLoadWorker.js new file mode 100644 index 000000000..a6c291263 --- /dev/null +++ b/test-app/app/src/main/assets/app/tests/workerTerminateDuringLoadWorker.js @@ -0,0 +1,5 @@ +// Spins at module scope so a terminate() from the parent lands while this +// module body is still executing, which is the window the test targets. +var deadline = Date.now() + 5000; +while (Date.now() < deadline) { +} diff --git a/test-app/runtime/src/main/cpp/CallbackHandlers.cpp b/test-app/runtime/src/main/cpp/CallbackHandlers.cpp index de1d0b345..28583568b 100644 --- a/test-app/runtime/src/main/cpp/CallbackHandlers.cpp +++ b/test-app/runtime/src/main/cpp/CallbackHandlers.cpp @@ -1496,12 +1496,14 @@ void CallbackHandlers::CallWorkerScopeOnErrorHandle(Isolate *isolate, TryCatch & auto globalObject = context->Global(); // execute onerror handle if one is implemented - auto callback = globalObject->Get(context, ArgConverter::ConvertToV8String(isolate, - "onerror")).ToLocalChecked(); - auto isEmpty = callback.IsEmpty(); + Local callback; + if (!globalObject->Get(context, ArgConverter::ConvertToV8String(isolate, "onerror")) + .ToLocal(&callback)) { + return; + } auto isFunction = callback->IsFunction(); - if (!isEmpty && isFunction && !tc.Message().IsEmpty()) { + if (isFunction && !tc.Message().IsEmpty()) { auto msg = tc.Message()->Get(); Local args1[] = {msg}; diff --git a/test-app/runtime/src/main/cpp/ModuleInternal.cpp b/test-app/runtime/src/main/cpp/ModuleInternal.cpp index ef9a2faae..86b3e3a47 100644 --- a/test-app/runtime/src/main/cpp/ModuleInternal.cpp +++ b/test-app/runtime/src/main/cpp/ModuleInternal.cpp @@ -374,10 +374,11 @@ Local ModuleInternal::LoadModule(Isolate* isolate, const string& moduleP if (Util::EndsWith(modulePath, ".js")) { auto script = LoadScript(isolate, modulePath, fullRequiredModulePath); - moduleFunc = script->Run(context).ToLocalChecked().As(); - if (tc.HasCaught()) { + Local moduleFuncValue; + if (!script->Run(context).ToLocal(&moduleFuncValue) || tc.HasCaught()) { throw NativeScriptException(tc, "Error running script " + modulePath); } + moduleFunc = moduleFuncValue.As(); } else if (Util::EndsWith(modulePath, ".so")) { auto handle = dlopen(modulePath.c_str(), RTLD_LAZY); if (handle == nullptr) { @@ -425,7 +426,11 @@ Local ModuleInternal::LoadModule(Isolate* isolate, const string& moduleP auto thiz = Object::New(isolate); auto extendsName = ArgConverter::ConvertToV8String(isolate, "__extends"); - thiz->Set(context, extendsName, context->Global()->Get(context, extendsName).ToLocalChecked()); + Local extendsFunc; + if (!context->Global()->Get(context, extendsName).ToLocal(&extendsFunc) || tc.HasCaught()) { + throw NativeScriptException(tc, "Cannot read '__extends' while loading " + modulePath); + } + thiz->Set(context, extendsName, extendsFunc); moduleFunc->Call(context, thiz, sizeof(requireArgs) / sizeof(Local ), requireArgs); if (tc.HasCaught()) { diff --git a/test-app/runtime/src/main/cpp/NativeScriptException.cpp b/test-app/runtime/src/main/cpp/NativeScriptException.cpp index 2f02ba685..03ddce967 100644 --- a/test-app/runtime/src/main/cpp/NativeScriptException.cpp +++ b/test-app/runtime/src/main/cpp/NativeScriptException.cpp @@ -44,8 +44,19 @@ NativeScriptException::NativeScriptException(TryCatch& tc, const string& message) : m_javaException(JniLocalRef()) { auto isolate = Isolate::GetCurrent(); - m_javascriptException = new Persistent(isolate, tc.Exception()); auto ex = tc.Exception(); + m_javascriptException = + ex.IsEmpty() ? nullptr : new Persistent(isolate, ex); + + // A terminated isolate carries no message object and no inspectable + // exception - every accessor below hands back an empty handle. Resetting + // does not cancel the isolate's pending termination. + if (tc.HasTerminated() || tc.Message().IsEmpty()) { + m_message = message.empty() ? "Execution terminated." : message; + tc.Reset(); + return; + } + m_message = GetErrorMessage(tc.Message(), ex, message); m_stackTrace = GetErrorStackTrace(tc.Message()->GetStackTrace()); m_fullMessage = GetFullMessage(tc, m_message); diff --git a/test-app/runtime/src/main/cpp/Runtime.cpp b/test-app/runtime/src/main/cpp/Runtime.cpp index 04d106f6e..c24431731 100644 --- a/test-app/runtime/src/main/cpp/Runtime.cpp +++ b/test-app/runtime/src/main/cpp/Runtime.cpp @@ -110,8 +110,9 @@ void Runtime::Init(JavaVM* vm, void* reserved) { // handle SIGABRT/SIGSEGV only on API level > 20 as the handling is not so // efficient in older versions if (m_androidVersion > 20) { - struct sigaction action; + struct sigaction action = {}; action.sa_handler = SIG_handler; + sigemptyset(&action.sa_mask); sigaction(SIGABRT, &action, NULL); sigaction(SIGSEGV, &action, NULL); }