refactor: replace assert() with NS_CHECK/NS_DCHECK

test-app/runtime/CMakeLists.txt

@@ -195,6 +195,7 @@ add_library(
         src/main/cpp/ModuleBinding.cpp
         src/main/cpp/ModuleInternal.cpp
         src/main/cpp/ModuleInternalCallbacks.cpp
+        src/main/cpp/NativeScriptAssert.cpp
         src/main/cpp/NativeScriptException.cpp
         src/main/cpp/NativeScriptPlatform.cpp
         src/main/cpp/NsBuiltinModules.cpp

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

@@ -177,16 +177,16 @@ Local<Value> ArgConverter::ConvertFromJavaLong(Isolate* isolate, jlong value) {
 }
 
 int64_t ArgConverter::ConvertToJavaLong(Isolate* isolate, const Local<Value>& value) {
-    assert(!value.IsEmpty());
+    NS_DCHECK(!value.IsEmpty());
 
     auto obj = Local<Object>::Cast(value);
 
-    assert(!obj.IsEmpty());
+    NS_DCHECK(!obj.IsEmpty());
 
     auto context = isolate->GetCurrentContext();
     Local<Value> temp;
     bool success = obj->Get(context, V8StringConstants::GetValue(isolate)).ToLocal(&temp);
-    assert(success && !temp.IsEmpty());
+    NS_DCHECK(success && !temp.IsEmpty());
     auto valueProp = temp.As<Object>();
 
     string num = ConvertToString(valueProp->ToString(context).ToLocalChecked());

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

@@ -1,4 +1,5 @@
 #include "ArrayBufferHelper.h"
+#include "NativeScriptAssert.h"
 #include "ArgConverter.h"
 #include "NativeScriptException.h"
 #include <sstream>
@@ -65,7 +66,7 @@ void ArrayBufferHelper::CreateFromCallbackImpl(const FunctionCallbackInfo<Value>
 
     if (m_ByteBufferClass == nullptr) {
         m_ByteBufferClass = env.FindClass("java/nio/ByteBuffer");
-        assert(m_ByteBufferClass != nullptr);
+        NS_CHECK(m_ByteBufferClass != nullptr);
     }
 
     auto isByteBuffer = env.IsInstanceOf(obj, m_ByteBufferClass);
@@ -76,7 +77,7 @@ void ArrayBufferHelper::CreateFromCallbackImpl(const FunctionCallbackInfo<Value>
 
     if (m_isDirectMethodID == nullptr) {
         m_isDirectMethodID = env.GetMethodID(m_ByteBufferClass, "isDirect", "()Z");
-        assert(m_isDirectMethodID != nullptr);
+        NS_CHECK(m_isDirectMethodID != nullptr);
     }
 
     auto ret = env.CallBooleanMethod(obj, m_isDirectMethodID);
@@ -99,14 +100,14 @@ void ArrayBufferHelper::CreateFromCallbackImpl(const FunctionCallbackInfo<Value>
     } else {
         if (m_remainingMethodID == nullptr) {
             m_remainingMethodID = env.GetMethodID(m_ByteBufferClass, "remaining", "()I");
-            assert(m_remainingMethodID != nullptr);
+            NS_CHECK(m_remainingMethodID != nullptr);
         }
 
         int bufferRemainingSize = env.CallIntMethod(obj, m_remainingMethodID);
 
         if (m_getMethodID == nullptr) {
             m_getMethodID = env.GetMethodID(m_ByteBufferClass, "get", "([BII)Ljava/nio/ByteBuffer;");
-            assert(m_getMethodID != nullptr);
+            NS_CHECK(m_getMethodID != nullptr);
         }
 
         jbyteArray byteArray = env.NewByteArray(bufferRemainingSize);

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

@@ -1,4 +1,5 @@
 #include "ArrayHelper.h"
+#include "NativeScriptAssert.h"
 #include "ArgConverter.h"
 #include "NativeScriptException.h"
 #include "Runtime.h"
@@ -15,10 +16,10 @@ void ArrayHelper::Init(const Local<Context>& context) {
     JEnv env;
 
     RUNTIME_CLASS = env.FindClass("com/tns/Runtime");
-    assert(RUNTIME_CLASS != nullptr);
+    NS_CHECK(RUNTIME_CLASS != nullptr);
 
     CREATE_ARRAY_HELPER = env.GetStaticMethodID(RUNTIME_CLASS, "createArrayHelper", "(Ljava/lang/String;I)Ljava/lang/Object;");
-    assert(CREATE_ARRAY_HELPER != nullptr);
+    NS_CHECK(CREATE_ARRAY_HELPER != nullptr);
 
     auto isolate = v8::Isolate::GetCurrent();
     auto global = context->Global();

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

@@ -1,9 +1,14 @@
 #include "jni.h"
 #include "zip.h"
-#include <assert.h>
+#include "NativeScriptAssert.h"
 #include <libgen.h>
 #include <utime.h>
+#include <unistd.h>
 #include <sys/stat.h>
+#include <cerrno>
+#include <cstdio>
+#include <cstdlib>
+#include <string>
 #include "AssetExtractor.h"
 
 using namespace tns;
@@ -22,7 +27,9 @@ void AssetExtractor::ExtractAssets(JNIEnv* env, jobject obj, jstring apk, jstrin
     int err = 0;
     auto z = zip_open(strApk.c_str(), 0, &err);
 
-    assert(z != nullptr);
+    // Nothing downstream can run without the app's assets, and every call
+    // below dereferences the archive.
+    NS_CHECK(z != nullptr);
     zip_int64_t num = zip_get_num_entries(z, 0);
     struct zip_stat sb;
     struct zip_file* zf;
@@ -53,23 +60,86 @@ void AssetExtractor::ExtractAssets(JNIEnv* env, jobject obj, jstring apk, jstrin
                 mkdir_rec(dirFullname.c_str());
 
                 zf = zip_fopen_index(z, i, 0);
-                assert(zf != nullptr);
+                if (zf == nullptr) {
+                    DEBUG_WRITE_FORCE(
+                        "AssetExtractor: skipping '%s', it could not be opened "
+                        "inside the apk (%s)",
+                        sb.name, zip_strerror(z));
+                    continue;
+                }
 
-                auto fd = fopen(assetFullname.c_str(), "w");
+                /*
+                 * Extracted through a temporary and renamed once complete.
+                 * Writing in place would truncate a good asset up front, and a
+                 * read that fails partway would leave the remains behind: the
+                 * mtime comparison above keeps whatever is already on disk
+                 * unless the apk's copy is strictly newer, so a partial file
+                 * would never be replaced on a later launch.
+                 */
+                std::string tempFullname(assetFullname);
+                tempFullname.append(".ns-XXXXXX");
+
+                // Exclusive creation, so two extractions of the same entry
+                // cannot land on one temporary. mkstemp writes the name it
+                // settled on back into the buffer.
+                FILE* fd = nullptr;
+                int tempFd = mkstemp(tempFullname.data());
+                if (tempFd < 0) {
+                    DEBUG_WRITE_FORCE(
+                        "AssetExtractor: could not create a temporary for '%s' "
+                        "(errno %d)",
+                        assetFullname.c_str(), errno);
+                } else {
+                    fd = fdopen(tempFd, "w");
+                    if (fd == nullptr) {
+                        DEBUG_WRITE_FORCE(
+                            "AssetExtractor: could not open the temporary for "
+                            "'%s' (errno %d)",
+                            assetFullname.c_str(), errno);
+                        close(tempFd);
+                        remove(tempFullname.c_str());
+                    }
+                }
 
                 if (fd != nullptr) {
                     zip_int64_t sum = 0;
-                    while (sum != sb.size) {
+                    bool complete = true;
+                    while (sum != (zip_int64_t)sb.size) {
                         zip_int64_t len = zip_fread(zf, buf, sizeof(buf));
-                        assert(len > 0);
-
-                        fwrite(buf, 1, len, fd);
+                        // 0 means the entry ended early, -1 a read error. Both
+                        // leave the loop unable to advance, and -1 would reach
+                        // fwrite as a huge size_t.
+                        if (len <= 0) {
+                            DEBUG_WRITE_FORCE(
+                                "AssetExtractor: '%s' ended after %lld of %llu "
+                                "bytes (%s)",
+                                sb.name, (long long)sum,
+                                (unsigned long long)sb.size,
+                                zip_file_strerror(zf));
+                            complete = false;
+                            break;
+                        }
+
+                        if (fwrite(buf, 1, len, fd) != (size_t)len) {
+                            DEBUG_WRITE_FORCE(
+                                "AssetExtractor: could not write '%s' (errno %d)",
+                                assetFullname.c_str(), errno);
+                            complete = false;
+                            break;
+                        }
                         sum += len;
                     }
-                    fclose(fd);
-                    utimbuf t;
-                    t.modtime = sb.mtime;
-                    utime(assetFullname.c_str(), &t);
+
+                    complete = (fclose(fd) == 0) && complete;
+
+                    if (complete && rename(tempFullname.c_str(),
+                                           assetFullname.c_str()) == 0) {
+                        utimbuf t;
+                        t.modtime = sb.mtime;
+                        utime(assetFullname.c_str(), &t);
+                    } else {
+                        remove(tempFullname.c_str());
+                    }
                 }
 
                 zip_fclose(zf);

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

@@ -1,4 +1,5 @@
 #include "CallbackHandlers.h"
+#include "NativeScriptAssert.h"
 #include "MetadataNode.h"
 #include "Util.h"
 #include "V8GlobalHelpers.h"
@@ -30,33 +31,33 @@ void CallbackHandlers::Init(Isolate *isolate) {
     JEnv env;
 
     JAVA_LANG_STRING = env.FindClass("java/lang/String");
-    assert(JAVA_LANG_STRING != nullptr);
+    NS_CHECK(JAVA_LANG_STRING != nullptr);
 
     RUNTIME_CLASS = env.FindClass("com/tns/Runtime");
-    assert(RUNTIME_CLASS != nullptr);
+    NS_CHECK(RUNTIME_CLASS != nullptr);
 
     RESOLVE_CLASS_METHOD_ID = env.GetMethodID(RUNTIME_CLASS, "resolveClass",
                                               "(Ljava/lang/String;Ljava/lang/String;[Ljava/lang/String;[Ljava/lang/String;Z)Ljava/lang/Class;");
-    assert(RESOLVE_CLASS_METHOD_ID != nullptr);
+    NS_CHECK(RESOLVE_CLASS_METHOD_ID != nullptr);
 
     CURRENT_OBJECTID_FIELD_ID = env.GetFieldID(RUNTIME_CLASS, "currentObjectId", "I");
-    assert(CURRENT_OBJECTID_FIELD_ID != nullptr);
+    NS_CHECK(CURRENT_OBJECTID_FIELD_ID != nullptr);
 
     MAKE_INSTANCE_STRONG_ID = env.GetMethodID(RUNTIME_CLASS, "makeInstanceStrong",
                                               "(Ljava/lang/Object;I)V");
-    assert(MAKE_INSTANCE_STRONG_ID != nullptr);
+    NS_CHECK(MAKE_INSTANCE_STRONG_ID != nullptr);
 
     GET_TYPE_METADATA = env.GetStaticMethodID(RUNTIME_CLASS, "getTypeMetadata",
                                               "(Ljava/lang/String;I)[Ljava/lang/String;");
-    assert(GET_TYPE_METADATA != nullptr);
+    NS_CHECK(GET_TYPE_METADATA != nullptr);
 
     ENABLE_VERBOSE_LOGGING_METHOD_ID = env.GetMethodID(RUNTIME_CLASS, "enableVerboseLogging",
                                                        "()V");
-    assert(ENABLE_VERBOSE_LOGGING_METHOD_ID != nullptr);
+    NS_CHECK(ENABLE_VERBOSE_LOGGING_METHOD_ID != nullptr);
 
     DISABLE_VERBOSE_LOGGING_METHOD_ID = env.GetMethodID(RUNTIME_CLASS, "disableVerboseLogging",
                                                         "()V");
-    assert(ENABLE_VERBOSE_LOGGING_METHOD_ID != nullptr);
+    NS_CHECK(DISABLE_VERBOSE_LOGGING_METHOD_ID != nullptr);
 
     MetadataNode::Init(isolate);
 
@@ -559,7 +560,7 @@ void CallbackHandlers::CallJavaMethod(const Local<Object> &caller, const string
             break;
         }
         default: {
-            assert(false);
+            NS_DCHECK(false);
             break;
         }
     }
@@ -674,7 +675,7 @@ CallbackHandlers::GetMethodOverrides(JEnv &env, const Local<Object> &implementat
 }
 
 void CallbackHandlers::RunOnMainThreadCallback(const FunctionCallbackInfo<v8::Value> &args) {
-    assert(args[0]->IsFunction());
+    NS_DCHECK(args[0]->IsFunction());
     Isolate *isolate = args.GetIsolate();
 
     v8::Locker locker(isolate);
@@ -695,7 +696,7 @@ void CallbackHandlers::RunOnMainThreadCallback(const FunctionCallbackInfo<v8::Va
         std::lock_guard<std::mutex> lock(cacheMutex_);
         bool inserted;
         std::tie(std::ignore, inserted) = cache_.try_emplace(key, isolate, callback);
-        assert(inserted && "Main thread callback ID should not be duplicated");
+        NS_DCHECK(inserted && "Main thread callback ID should not be duplicated");
     }
 
     // bare entry: the closure locks the CALLER's isolate (possibly a
@@ -957,7 +958,7 @@ vector<string> CallbackHandlers::GetTypeMetadata(const string &name, int index)
 
     jsize length = env.GetArrayLength(pubApi);
 
-    assert(length > 0);
+    NS_DCHECK(length > 0);
 
     vector<string> result;