fix: make LiveSync writes atomic instead of locking the runtime's reads

m_fileWriteMutex paired LiveSync's file writes against Runtime::ReadFileText so
the runtime would not compile a half-written hot-synced module. It could not
deliver that, for three independent reasons:

- It is a non-static member, so LiveSync locking one runtime excludes nothing on
  a worker's thread - precisely the case that matters now that workers run on
  their own detached threads.
- The lock was taken after prepareFile() deleted the file and the
  FileOutputStream constructor truncated it, so it covered only fos.write and
  not the window a reader actually hits.
- The finally block called unlock() even when lock() had never been reached (a
  throw from mkdirs() or the FileOutputStream constructor). Unlocking a
  std::mutex this thread does not own is undefined behaviour, and on bionic can
  release a lock the JS thread holds inside ReadFileText.

Writes now land through a sibling temp file renamed over the target. rename(2)
within a directory is atomic, so a reader gets either the whole previous file or
the whole new one, on every runtime and every thread, with no shared state to
scope wrongly. That also closes the delete/truncate window, which no reader-side
lock could have covered, and takes a mutex off every debug module read.

java.nio.file's ATOMIC_MOVE is unavailable at minSdk 21, so this uses
File.createTempFile in the target's own directory plus File.renameTo, which maps
to rename(2) on every API level. The temp file must be a sibling: rename is only
atomic within one filesystem.

m_fileWriteMutex, Runtime::Lock/Unlock, both lock_guards, the two JNI entry
points and the private native lock/unlock declarations are removed.
com.tns.Runtime.lock()/unlock() remain as deprecated no-ops: they are public API
and external tooling may call them.

The test suite does not exercise LiveSync, so this is verified by the arm64-v8a
build and by the suite not regressing (1038/1038). Hot-sync itself needs a
manual check.

Author: edusperoni

test-app/app/src/debug/java/com/tns/NativeScriptSyncServiceSocketImpl.java

@@ -357,23 +357,45 @@ private int getLength() {
             return lengthInt;
         }
 
+        /*
+         * Written through a sibling temp file renamed over the target, because
+         * the app keeps running JS while files stream in: rename(2) within a
+         * directory is atomic, so a runtime thread that requires this path
+         * mid-sync gets either the whole previous file or the whole new one.
+         * Writing in place cannot be made safe from the reader side -- the
+         * truncate lands before any lock a reader could share, and each runtime
+         * (main, every worker) reads on its own thread.
+         */
         private void createOrOverrideFile(String fileName, byte[] content) throws IOException {
-            File fileToCreate = prepareFile(fileName);
-            try {
+            File fileToCreate = new File(DEVICE_APP_DIR, fileName);
+            File parentDir = fileToCreate.getParentFile();
 
-                fileToCreate.getParentFile().mkdirs();
-                FileOutputStream fos = new FileOutputStream(fileToCreate.getCanonicalPath());
-                if(runtime != null) {
-                    runtime.lock();
+            if (parentDir != null) {
+                parentDir.mkdirs();
+            }
+
+            File temp = null;
+            try {
+                // Same directory as the target: rename is only atomic within one
+                // filesystem, and a sibling is the one placement that guarantees it.
+                temp = File.createTempFile("livesync", ".tmp", parentDir);
+
+                FileOutputStream fos = new FileOutputStream(temp);
+                try {
+                    fos.write(content);
+                } finally {
+                    fos.close();
                 }
-                fos.write(content);
-                fos.close();
 
+                if (!temp.renameTo(fileToCreate)) {
+                    throw new IOException(String.format("failed to rename %s onto the target", temp.getAbsolutePath()));
+                }
+                temp = null;
             } catch (Exception e) {
                 throw new IOException(String.format("\nLiveSync: failed to write file: %s\nOriginal Exception: %s", fileName, e.toString()));
             } finally {
-                if(runtime != null) {
-                    runtime.unlock();
+                if (temp != null) {
+                    temp.delete();
                 }
             }
         }
@@ -387,14 +409,6 @@ void deleteRecursive(File fileOrDirectory) {
             fileOrDirectory.delete();
         }
 
-        private File prepareFile(String fileName) {
-            File fileToCreate = new File(DEVICE_APP_DIR, fileName);
-            if (fileToCreate.exists()) {
-                fileToCreate.delete();
-            }
-            return fileToCreate;
-        }
-
         /*
         * Reads next bites from input stream. Bytes read depend on passed parameter.
         * */

test-app/runtime/src/main/cpp/Runtime.cpp

@@ -335,31 +335,13 @@ Runtime::~Runtime() {
 }
 
 std::string Runtime::ReadFileText(const std::string& filePath) {
-#ifdef APPLICATION_IN_DEBUG
-  std::lock_guard<std::mutex> lock(m_fileWriteMutex);
-#endif
   return File::ReadText(filePath);
 }
 
 std::string Runtime::ReadFileText(const std::string& filePath, bool& ok) {
-#ifdef APPLICATION_IN_DEBUG
-  std::lock_guard<std::mutex> lock(m_fileWriteMutex);
-#endif
   return File::ReadText(filePath, ok);
 }
 
-void Runtime::Lock() {
-#ifdef APPLICATION_IN_DEBUG
-  m_fileWriteMutex.lock();
-#endif
-}
-
-void Runtime::Unlock() {
-#ifdef APPLICATION_IN_DEBUG
-  m_fileWriteMutex.unlock();
-#endif
-}
-
 // The boot backstop: hold the launching thread until boot has actually
 // finished. Two independent things can leave it unfinished, and BOTH must hold
 // the pump — an in-flight module-graph load, and an entry whose own evaluation

test-app/runtime/src/main/cpp/Runtime.h

@@ -145,9 +145,6 @@ class Runtime {
         jboolean PassExceptionToJsNative(JNIEnv* env, jobject obj, jthrowable exception, jstring message, jstring fullStackTrace, jstring jsStackTrace, jboolean isDiscarded);
         void DestroyRuntime();
 
-        void Lock();
-        void Unlock();
-
         int GetId();
 
         v8::Local<v8::Context> GetContext();
@@ -392,9 +389,6 @@ class Runtime {
 
         static thread_local Runtime* s_currentRuntime;
 
-#ifdef APPLICATION_IN_DEBUG
-        std::mutex m_fileWriteMutex;
-#endif
 };
 }
 

test-app/runtime/src/main/cpp/com_tns_Runtime.cpp

@@ -285,20 +285,6 @@ extern "C" JNIEXPORT jboolean Java_com_tns_Runtime_notifyGcLegacy(JNIEnv* env, j
     return notifyGcFast_impl(env, obj, runtimeId);
 }
 
-extern "C" JNIEXPORT void Java_com_tns_Runtime_lock(JNIEnv* env, jobject obj, jint runtimeId) {
-    auto runtime = TryGetRuntime(runtimeId);
-    if (runtime != nullptr) {
-        runtime->Lock();
-    }
-}
-
-extern "C" JNIEXPORT void Java_com_tns_Runtime_unlock(JNIEnv* env, jobject obj, jint runtimeId) {
-    auto runtime = TryGetRuntime(runtimeId);
-    if (runtime != nullptr) {
-        runtime->Unlock();
-    }
-}
-
 extern "C" JNIEXPORT jboolean Java_com_tns_Runtime_passExceptionToJsNative(JNIEnv* env, jobject obj, jint runtimeId, jthrowable exception, jstring message, jstring fullStackTrace, jstring jsStackTrace, jboolean isDiscarded) {
     auto runtime = TryGetRuntime(runtimeId);
     if (runtime == nullptr) {

test-app/runtime/src/main/java/com/tns/Runtime.java

@@ -71,10 +71,6 @@ private boolean notifyGc(int runtimeId) {
         return SUPPORTS_OPTIMIZED_NATIVE ? notifyGcFast(runtimeId) : notifyGcLegacy(runtimeId);
     }
 
-    private native void lock(int runtimeId);
-
-    private native void unlock(int runtimeId);
-
     private native boolean passExceptionToJsNative(int runtimeId, Throwable ex, String message, String fullStackTrace, String jsStackTrace, boolean isDiscarded);
 
     @CriticalNative
@@ -766,12 +762,20 @@ public void notifyGc() {
         notifyGc(runtimeId);
     }
 
+    /**
+     * @deprecated No-op. This paired the runtime's file reads against LiveSync's
+     * writes, which now land atomically via a temp file renamed over the target,
+     * so readers need no lock. Retained because it is public API.
+     */
+    @Deprecated
     public void lock() {
-        lock(runtimeId);
     }
 
+    /**
+     * @deprecated No-op. See {@link #lock()}.
+     */
+    @Deprecated
     public void unlock() {
-        unlock(runtimeId);
     }
 
     public static void initInstanceFromPossibleNonMainThread(final Object instance) {