Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions test-app/app/src/main/java/com/tns/NativeScriptRuntime.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.tns;

public final class NativeScriptRuntime {
private NativeScriptRuntime() {
}

public static boolean reloadApplication() {
return RuntimeHelper.reloadApplication();
}

public static boolean reloadApplication(String baseDir) {
return RuntimeHelper.reloadApplication();
Comment on lines +11 to +12

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Wire baseDir through or remove this overload.

reloadApplication(String baseDir) currently ignores baseDir, so callers cannot select an OTA/new bundle location even though the public API suggests they can.

Possible direction
 public static boolean reloadApplication(String baseDir) {
-    return RuntimeHelper.reloadApplication();
+    return RuntimeHelper.reloadApplication(baseDir);
 }

Also add the corresponding RuntimeHelper.reloadApplication(String baseDir) implementation that rebuilds runtime configuration from that directory.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test-app/app/src/main/java/com/tns/NativeScriptRuntime.java` around lines 11
- 12, The NativeScriptRuntime.reloadApplication(String baseDir) overload
currently drops its baseDir argument, so callers cannot reload from a chosen
bundle location. Update this method to either pass baseDir through to a new
RuntimeHelper.reloadApplication(String baseDir) implementation that rebuilds the
runtime config from that directory, or remove the overload if it is not meant to
be supported; keep the API behavior aligned with the method signature and
related RuntimeHelper reload logic.

}
}
53 changes: 52 additions & 1 deletion test-app/app/src/main/java/com/tns/RuntimeHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import android.content.SharedPreferences;
import android.content.pm.PackageManager.NameNotFoundException;
import android.os.Build;
import android.os.Handler;
import android.os.Looper;
import android.preference.PreferenceManager;
import android.util.Log;

Expand All @@ -24,6 +26,9 @@ private RuntimeHelper() {
}

private static AndroidJsV8Inspector v8Inspector;
private static Context applicationContext;
private static BroadcastReceiver timezoneChangedReceiver;
private static boolean reloadScheduled;

// hasErrorIntent tells you if there was an event (with an uncaught
// exception) raised from ErrorReport
Expand Down Expand Up @@ -59,6 +64,9 @@ private static boolean hasErrorIntent(Context context) {
}

public static Runtime initRuntime(Context context) {
Context appContext = context.getApplicationContext();
applicationContext = appContext != null ? appContext : context;

if (Runtime.isInitialized()) {
return Runtime.getCurrentRuntime();
}
Expand Down Expand Up @@ -237,6 +245,40 @@ public static Runtime initRuntime(Context context) {
}
}

public static synchronized boolean reloadApplication() {
final Context context = applicationContext;
if (context == null || reloadScheduled) {
return false;
}

reloadScheduled = true;

new Handler(Looper.getMainLooper()).post(new Runnable() {
@Override
public void run() {
try {
Runtime.destroyMainRuntime();

Runtime runtime = initRuntime(context);
if (runtime == null) {
throw new IllegalStateException("NativeScript runtime reload failed to initialize a new runtime.");
}

runtime.run();
} catch (Throwable e) {
Log.e(logTag, "NativeScript runtime reload failed.", e);
throw new RuntimeException("NativeScript runtime reload failed.", e);
} finally {
synchronized (RuntimeHelper.class) {
reloadScheduled = false;
}
}
}
});

return true;
}

private static void waitForLiveSync(Context context) {
boolean needToWait = false;

Expand Down Expand Up @@ -295,7 +337,16 @@ public void onReceive(Context context, Intent intent) {
}
};

context.registerReceiver(timezoneReceiver, timezoneFilter);
if (timezoneChangedReceiver != null) {
try {
context.unregisterReceiver(timezoneChangedReceiver);
} catch (IllegalArgumentException e) {
// Already unregistered.
}
}

timezoneChangedReceiver = timezoneReceiver;
context.registerReceiver(timezoneChangedReceiver, timezoneFilter);
Comment on lines +340 to +349

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

head -n 500 test-app/app/src/main/java/com/tns/RuntimeHelper.java

Repository: NativeScript/android

Length of output: 17371


🏁 Script executed:

tail -n +50 test-app/app/src/main/java/com/tns/RuntimeHelper.java | grep -n "registerTimezoneChangedListener"

Repository: NativeScript/android

Length of output: 335


Receiver unregistration fails on reload due to Context mismatch.

registerTimezoneChangedListener is called initially with a passed context (often Activity), but during reload via reloadApplication, it is called with applicationContext. Since unregisterReceiver requires the same Context instance used for registration, the attempt to unregister the previous receiver fails, hiding the error in the catch block. This leaves the old receiver registered, leading to listener leaks or potential crashes when multiple receivers are active.

Suggested fix
+private static Context timezoneChangedReceiverContext;
 ...
-        if (timezoneChangedReceiver != null) {
+        Context receiverContext = context.getApplicationContext() != null ? context.getApplicationContext() : context;
+        if (timezoneChangedReceiver != null && timezoneChangedReceiverContext != null) {
             try {
-                context.unregisterReceiver(timezoneChangedReceiver);
+                timezoneChangedReceiverContext.unregisterReceiver(timezoneChangedReceiver);
             } catch (IllegalArgumentException e) {
                 // Already unregistered.
             }
         }
 
         timezoneChangedReceiver = timezoneReceiver;
+        timezoneChangedReceiverContext = receiverContext;
         context.registerReceiver(timezoneChangedReceiver, timezoneFilter);
+        context = receiverContext; // Use consistent context scope for subsequent logic if needed
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test-app/app/src/main/java/com/tns/RuntimeHelper.java` around lines 340 -
349, `registerTimezoneChangedListener` is unregistering
`timezoneChangedReceiver` with whatever `context` is passed in, but that can
differ between the initial call and `reloadApplication`, causing the old
receiver to remain registered. Update the listener bookkeeping so the same
Context instance used for registration is also used for `unregisterReceiver`
(for example by storing the registering Context alongside
`timezoneChangedReceiver` or consistently using `applicationContext` in
`registerTimezoneChangedListener` and its callers). Keep the existing
`IllegalArgumentException` handling only for truly stale registrations, and
ensure the old receiver is fully removed before assigning and registering the
new one.

}

public static void initLiveSync(Application app) {
Expand Down
123 changes: 122 additions & 1 deletion test-app/runtime/src/main/cpp/com_tns_Runtime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,127 @@ extern "C" JNIEXPORT jint Java_com_tns_Runtime_getCurrentRuntimeIdLegacy(JNIEnv*
return getCurrentRuntimeIdCritical_impl();
}

extern "C" JNIEXPORT void Java_com_tns_Runtime_WorkerGlobalOnMessageCallback(JNIEnv* env, jobject obj, jint runtimeId, jstring msg) {
// Worker Thread runtime
auto runtime = TryGetRuntime(runtimeId);
if (runtime == nullptr) {
// TODO: Pete: Log message informing the developer of the failure
}

auto isolate = runtime->GetIsolate();
Comment on lines +358 to +362

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Return when runtime lookup fails.

These callbacks detect runtime == nullptr but continue and immediately dereference runtime via runtime->GetIsolate(), causing a native crash for stale/invalid runtime IDs.

Proposed fix
     auto runtime = TryGetRuntime(runtimeId);
     if (runtime == nullptr) {
         // TODO: Pete: Log message informing the developer of the failure
+        return;
     }

Apply the same return to each null-runtime branch in the affected callbacks.

Also applies to: 376-380, 441-445, 459-463

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test-app/runtime/src/main/cpp/com_tns_Runtime.cpp` around lines 358 - 362,
The null-runtime checks in the affected callbacks do not stop execution, so
`runtime->GetIsolate()` can still be called on a null pointer and crash. In each
callback that resolves a `runtime` and then uses it (`com_tns_Runtime` helpers
around the reported branches), add an immediate return inside the `if (runtime
== nullptr)` path before any dereference, and apply the same early-exit pattern
consistently to all listed null-runtime branches.


v8::Locker locker(isolate);
v8::Isolate::Scope isolate_scope(isolate);
v8::HandleScope handleScope(isolate);
auto context = runtime->GetContext();
v8::Context::Scope context_scope(context);

CallbackHandlers::WorkerGlobalOnMessageCallback(isolate, msg);
}

extern "C" JNIEXPORT void Java_com_tns_Runtime_WorkerObjectOnMessageCallback(JNIEnv* env, jobject obj, jint runtimeId, jint workerId, jstring msg) {
// Main Thread runtime
auto runtime = TryGetRuntime(runtimeId);
if (runtime == nullptr) {
// TODO: Pete: Log message informing the developer of the failure
}

auto isolate = runtime->GetIsolate();

v8::Locker locker(isolate);
v8::Isolate::Scope isolate_scope(isolate);
v8::HandleScope handleScope(isolate);
auto context = runtime->GetContext();
v8::Context::Scope context_scope(context);

CallbackHandlers::WorkerObjectOnMessageCallback(isolate, workerId, msg);
}

extern "C" JNIEXPORT void Java_com_tns_Runtime_TerminateWorkerCallback(JNIEnv* env, jobject obj, jint runtimeId) {
// Worker Thread runtime
auto runtime = TryGetRuntime(runtimeId);
if (runtime == nullptr) {
// TODO: Pete: Log message informing the developer of the failure
return;
}

auto isolate = runtime->GetIsolate();

{
v8::Locker locker(isolate);
v8::Isolate::Scope isolate_scope(isolate);
v8::HandleScope handleScope(isolate);

CallbackHandlers::TerminateWorkerThread(isolate);

runtime->DestroyRuntime();
}

isolate->Dispose();

delete runtime;
}

extern "C" JNIEXPORT void Java_com_tns_Runtime_TerminateRuntimeCallback(JNIEnv* env, jobject obj, jint runtimeId) {
auto runtime = TryGetRuntime(runtimeId);
if (runtime == nullptr) {
// TODO: Pete: Log message informing the developer of the failure
return;
}

auto isolate = runtime->GetIsolate();

{
v8::Locker locker(isolate);
v8::Isolate::Scope isolate_scope(isolate);
v8::HandleScope handleScope(isolate);

runtime->DestroyRuntime();
}

isolate->Dispose();

delete runtime;
}

extern "C" JNIEXPORT void Java_com_tns_Runtime_ClearWorkerPersistent(JNIEnv* env, jobject obj, jint runtimeId, jint workerId) {
// Worker Thread runtime
auto runtime = TryGetRuntime(runtimeId);
if (runtime == nullptr) {
// TODO: Pete: Log message informing the developer of the failure
}

auto isolate = runtime->GetIsolate();

v8::Locker locker(isolate);
v8::Isolate::Scope isolate_scope(isolate);
v8::HandleScope handleScope(isolate);
auto context = runtime->GetContext();
v8::Context::Scope context_scope(context);

CallbackHandlers::ClearWorkerPersistent(workerId);
}

extern "C" JNIEXPORT void Java_com_tns_Runtime_CallWorkerObjectOnErrorHandleMain(JNIEnv* env, jobject obj, jint runtimeId, jint workerId, jstring message, jstring stackTrace, jstring filename, jint lineno, jstring threadName) {
// Main Thread runtime
auto runtime = TryGetRuntime(runtimeId);
if (runtime == nullptr) {
// TODO: Pete: Log message informing the developer of the failure
}

auto isolate = runtime->GetIsolate();
v8::Locker locker(isolate);
v8::Isolate::Scope isolate_scope(isolate);
v8::HandleScope handleScope(isolate);
auto context = runtime->GetContext();
v8::Context::Scope context_scope(context);

try {
CallbackHandlers::CallWorkerObjectOnErrorHandle(isolate, workerId, message, stackTrace, filename, lineno, threadName);
} catch (NativeScriptException& e) {
e.ReThrowToJava();
}
}
extern "C" JNIEXPORT void Java_com_tns_Runtime_ResetDateTimeConfigurationCache(JNIEnv* _env, jobject obj, jint runtimeId) {
auto runtime = TryGetRuntime(runtimeId);
if (runtime == nullptr) {
Expand All @@ -360,4 +481,4 @@ extern "C" JNIEXPORT void Java_com_tns_Runtime_ResetDateTimeConfigurationCache(J

auto isolate = runtime->GetIsolate();
isolate->DateTimeConfigurationChangeNotification(Isolate::TimeZoneDetection::kRedetect);
}
}
45 changes: 45 additions & 0 deletions test-app/runtime/src/main/java/com/tns/Runtime.java
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,17 @@ public static void SetManualInstrumentationMode(String mode) {
}
}

private static native void WorkerGlobalOnMessageCallback(int runtimeId, String message);

private static native void WorkerObjectOnMessageCallback(int runtimeId, int workerId, String message);

private static native void TerminateWorkerCallback(int runtimeId);

private static native void TerminateRuntimeCallback(int runtimeId);

private static native void ClearWorkerPersistent(int runtimeId, int workerId);

private static native void CallWorkerObjectOnErrorHandleMain(int runtimeId, int workerId, String message, String stackTrace, String filename, int lineno, String threadName) throws NativeScriptException;
private static native void ResetDateTimeConfigurationCache(int runtimeId);

void passUncaughtExceptionToJs(Throwable ex, String message, String fullStackTrace, String jsStackTrace) {
Expand Down Expand Up @@ -454,6 +465,40 @@ public static boolean isInitialized() {
return (runtime != null) ? runtime.isInitializedImpl() : false;
}

static void destroyMainRuntime() {
Runtime runtime = Runtime.getCurrentRuntime();
if (runtime == null) {
return;
}

if (runtime.workerId != 0) {
throw new NativeScriptException("Only the main NativeScript runtime can be destroyed with destroyMainRuntime().");
}

runtime.isTerminating = true;
runtime.terminateWorkers();
GcListener.unsubscribe(runtime);
runtimeCache.remove(runtime.runtimeId);
pendingWorkerMessages.clear();
currentRuntime.remove();

TerminateRuntimeCallback(runtime.runtimeId);
}

private void terminateWorkers() {
for (Handler workerHandler : new ArrayList<>(workerIdToHandler.values())) {
if (workerHandler == null) {
continue;
}

Message msg = Message.obtain();
msg.arg1 = MessageType.TerminateThread;
workerHandler.sendMessageAtFrontOfQueue(msg);
}

workerIdToHandler.clear();
}
Comment on lines +478 to +500

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether worker termination is acknowledged before main runtime disposal.
rg -n -C4 '\bterminateWorkers\b|MessageType\.TerminateThread|workerIdToHandler|TerminateRuntimeCallback|detachWorkerRuntime' test-app/runtime/src/main/java/com/tns/Runtime.java

Repository: NativeScript/android

Length of output: 2140


Enforce block until worker termination completes before clearing main runtime state

terminateWorkers() sends a TerminateThread message asynchronously and immediately clears workerIdToHandler. Consequently, destroyMainRuntime() removes the runtime from the cached map and invokes TerminateRuntimeCallback while worker threads may still be processing their message queue. This race window allows worker code to reference a runtime instance that has been logically destroyed or accessed via stale handler references.

Insert a synchronization mechanism (e.g., wait, CountDownLatch, or Looper drain) to ensure worker threads have fully processed the termination message before the main runtime state is cleared.

Risk zone
    // test-app/runtime/src/main/java/com/tns/Runtime.java:478-500

        runtime.isTerminating = true;
        runtime.terminateWorkers(); // Sends signal but returns immediately
        GcListener.unsubscribe(runtime);
        runtimeCache.remove(runtime.runtimeId); // Runtime effectively destroyed here
        pendingWorkerMessages.clear();
        currentRuntime.remove();

        TerminateRuntimeCallback(runtime.runtimeId);
    }

    private void terminateWorkers() {
        for (Handler workerHandler : new ArrayList<>(workerIdToHandler.values())) {
            if (workerHandler == null) {
                continue;
            }

            Message msg = Message.obtain();
            msg.arg1 = MessageType.TerminateThread;
            workerHandler.sendMessageAtFrontOfQueue(msg);
        }

        workerIdToHandler.clear(); // Map cleared before worker processes msg
    }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test-app/runtime/src/main/java/com/tns/Runtime.java` around lines 478 - 500,
The termination flow in Runtime.destroyMainRuntime() and terminateWorkers() is
racing because the TerminateThread message is posted asynchronously and
workerIdToHandler is cleared immediately afterward. Add a blocking handshake so
terminateWorkers() waits until each worker has actually processed the
termination signal before destroyMainRuntime() continues with
runtimeCache.remove(), pendingWorkerMessages.clear(), currentRuntime.remove(),
and TerminateRuntimeCallback. Use the existing Runtime and workerHandler
messaging path to coordinate completion, and only clear workerIdToHandler after
all workers confirm shutdown.


public int getWorkerId() {
return workerId;
}
Expand Down
Loading