-
-
Notifications
You must be signed in to change notification settings - Fork 142
feat: reloadApplication for JS bundle restart without restarting app process #1963
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
||
|
|
@@ -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 | ||
|
|
@@ -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(); | ||
| } | ||
|
|
@@ -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; | ||
|
|
||
|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.javaRepository: 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.
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 |
||
| } | ||
|
|
||
| public static void initLiveSync(Application app) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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 |
||
|
|
||
| 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) { | ||
|
|
@@ -360,4 +481,4 @@ extern "C" JNIEXPORT void Java_com_tns_Runtime_ResetDateTimeConfigurationCache(J | |
|
|
||
| auto isolate = runtime->GetIsolate(); | ||
| isolate->DateTimeConfigurationChangeNotification(Isolate::TimeZoneDetection::kRedetect); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) { | ||
|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.javaRepository: NativeScript/android Length of output: 2140 Enforce block until worker termination completes before clearing main runtime state
Insert a synchronization mechanism (e.g., 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 |
||
|
|
||
| public int getWorkerId() { | ||
| return workerId; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
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
baseDirthrough or remove this overload.reloadApplication(String baseDir)currently ignoresbaseDir, 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