Fix JNI string lifetime/null-safety, Windows cancel-drain race, and UWP version parse

lib/http/HttpClient_WinInet.cpp

@@ -534,11 +534,21 @@ void HttpClient_WinInet::CancelAllRequests()
     for (const auto &id : ids)
         CancelRequestAsync(id);
 
-    // wait for all destructors to run
-    while (!m_requests.empty())
+    // wait for all destructors to run. Read m_requests under the lock each
+    // iteration; erase() runs on the WinInet callback thread under the same lock.
+    bool done;
+    {
+        LOCKGUARD(m_requestsMutex);
+        done = m_requests.empty();
+    }
+    while (!done)
     {
         PAL::sleep(100);
         std::this_thread::yield();
+        {
+            LOCKGUARD(m_requestsMutex);
+            done = m_requests.empty();
+        }
     }
 }
 

lib/http/HttpClient_WinRt.cpp

@@ -351,11 +351,21 @@ namespace MAT_NS_BEGIN {
         for (const auto &id : ids)
             CancelRequestAsync(id);
 
-        // wait for all destructors to run
-        while (!m_requests.empty())
+        // wait for all destructors to run. Read m_requests under the lock each
+        // iteration; erase() runs on the PPL continuation thread under the same lock.
+        bool done;
+        {
+            std::lock_guard<std::mutex> lock(m_requestsMutex);
+            done = m_requests.empty();
+        }
+        while (!done)
         {
             PAL::sleep(100);
             std::this_thread::yield();
+            {
+                std::lock_guard<std::mutex> lock(m_requestsMutex);
+                done = m_requests.empty();
+            }
         }
     };
 

lib/jni/JniConvertors.cpp

@@ -13,6 +13,8 @@ std::string JStringToStdString(JNIEnv* env, const jstring& jstr) {
 
     size_t jstr_length = env->GetStringUTFLength(jstr);
     auto jstr_utf = env->GetStringUTFChars(jstr, nullptr);
+    if (jstr_utf == nullptr)
+        return "";
     std::string str(jstr_utf, jstr_utf + jstr_length);
     env->ReleaseStringUTFChars(jstr, jstr_utf);
     return str;

lib/jni/Signals_jni.cpp

@@ -25,11 +25,19 @@ Java_com_microsoft_applications_events_Signals_sendSignal(JNIEnv *env,
                                                           jlong nativeLoggerPtr,
                                                           jstring signal_item_json) {
     jboolean isCopy = true;
+    if (signal_item_json == nullptr) {
+        return false;
+    }
     const char *signalItemJson = (env)->GetStringUTFChars(signal_item_json, &isCopy);
-    env->ReleaseStringUTFChars(signal_item_json, signalItemJson);
+    if (signalItemJson == nullptr) {
+        // GetStringUTFChars returned null (e.g. OOM, with a pending exception);
+        // there is nothing valid to log.
+        return false;
+    }
 
     auto logger = reinterpret_cast<ILogger*>(nativeLoggerPtr);
     EventProperties eventProperties = Signals::CreateEventProperties(signalItemJson);
+    env->ReleaseStringUTFChars(signal_item_json, signalItemJson);
     logger->LogEvent(eventProperties);
     return true;
 }
@@ -54,11 +62,18 @@ Java_com_microsoft_applications_events_Signals_nativeInitialize(JNIEnv *env, jcl
     SubstrateSignalsConfiguration config;
 
     jboolean isCopy = true;
-    const char *convertedValue = (env)->GetStringUTFChars(base_url, &isCopy);
-    if (strlen(convertedValue) > 0) {
-        config.ServiceRequestConfig.BaseUrl = convertedValue;
+    if (base_url != nullptr) {
+        const char *convertedValue = (env)->GetStringUTFChars(base_url, &isCopy);
+        if (convertedValue == nullptr) {
+            // GetStringUTFChars failed (e.g. OOM) and left a pending exception;
+            // do not continue making JNI calls with an exception in flight.
+            return false;
+        }
+        if (strlen(convertedValue) > 0) {
+            config.ServiceRequestConfig.BaseUrl = convertedValue;
+        }
+        env->ReleaseStringUTFChars(base_url, convertedValue);
     }
-    env->ReleaseStringUTFChars(base_url, convertedValue);
 
     config.ServiceRequestConfig.TimeoutMs = reinterpret_cast<int>(timeout_ms);
     config.ServiceRequestConfig.RetryTimes = reinterpret_cast<int>(retry_times);

lib/offline/OfflineStorage_Room.cpp

@@ -495,7 +495,9 @@ namespace MAT_NS_BEGIN
                     auto tenantToken_java = static_cast<jstring>(env->GetObjectField(record,
                                                                                      tenantToken_id));
                     ThrowRuntime(env, "get tenant");
-                    auto token_utf = env->GetStringUTFChars(tenantToken_java, nullptr);
+                    auto token_utf = (tenantToken_java != nullptr)
+                                         ? env->GetStringUTFChars(tenantToken_java, nullptr)
+                                         : nullptr;
                     ThrowRuntime(env, "string tenant");
                     auto latency = static_cast<EventLatency>(std::max(latency_lb,
                                                                       std::min<int>(
@@ -529,14 +531,17 @@ namespace MAT_NS_BEGIN
                     uint8_t* end = start + env->GetArrayLength(blob_java);
                     StorageRecord dest(
                         std::to_string(id_java),
-                        token_utf,
+                        token_utf != nullptr ? token_utf : "",
                         latency,
                         persistence,
                         timestamp,
                         StorageBlob(start, end),
                         retryCount,
                         reservedUntil);
-                    env->ReleaseStringUTFChars(tenantToken_java, token_utf);
+                    if (token_utf != nullptr)
+                    {
+                        env->ReleaseStringUTFChars(tenantToken_java, token_utf);
+                    }
                     env->ReleaseByteArrayElements(blob_java,
                                                   reinterpret_cast<jbyte*>(start), 0);
                     env.popLocalFrame();
@@ -769,10 +774,17 @@ namespace MAT_NS_BEGIN
                     ThrowLogic(env, "Exception fetching token");
                     auto count = env->GetLongField(byTenant, count_id);
                     ThrowLogic(env, "Exception fetching count");
-                    auto utf = env->GetStringUTFChars(token, nullptr);
-                    std::string key(utf);
-                    env->ReleaseStringUTFChars(token, utf);
-                    dropped[key] = static_cast<size_t>(count);
+                    auto utf = (token != nullptr) ? env->GetStringUTFChars(token, nullptr)
+                                                  : nullptr;
+                    ThrowRuntime(env, "Exception fetching token string");
+                    // Skip rather than misattribute dropped records to an empty
+                    // tenant token when the string read fails.
+                    if (utf != nullptr)
+                    {
+                        std::string key(utf);
+                        env->ReleaseStringUTFChars(token, utf);
+                        dropped[key] = static_cast<size_t>(count);
+                    }
                     env.popLocalFrame();
                 }
                 m_observer->OnStorageRecordsDropped(dropped);
@@ -1098,8 +1110,11 @@ namespace MAT_NS_BEGIN
             {
                 auto utf = env->GetStringUTFChars(java_value, nullptr);
                 ThrowRuntime(env, "copy setting value");
-                result = utf;
-                env->ReleaseStringUTFChars(java_value, utf);
+                if (utf != nullptr)
+                {
+                    result = utf;
+                    env->ReleaseStringUTFChars(java_value, utf);
+                }
             }
             return result;
         }
@@ -1343,7 +1358,9 @@ namespace MAT_NS_BEGIN
                 auto id_j = env->GetLongField(record, id_id);
                 auto tenant_j = static_cast<jstring>(env->GetObjectField(record,
                                                                          tenantToken_id));
-                auto tenant_utf = env->GetStringUTFChars(tenant_j, nullptr);
+                const char* tenant_utf = (tenant_j != nullptr)
+                                             ? env->GetStringUTFChars(tenant_j, nullptr)
+                                             : nullptr;
                 auto latency = static_cast<EventLatency>(env->GetIntField(record,
                                                                           latency_id));
                 auto persistence = static_cast<EventPersistence>(env->GetIntField(record,
@@ -1359,14 +1376,17 @@ namespace MAT_NS_BEGIN
                 auto blob_end = blob_store + blob_length;
                 records.emplace_back(
                     std::to_string(id_j),
-                    tenant_utf,
+                    tenant_utf != nullptr ? tenant_utf : "",
                     latency,
                     persistence,
                     timestamp,
                     StorageBlob(blob_store, blob_end),
                     retryCount,
                     reservedUntil);
-                env->ReleaseStringUTFChars(tenant_j, tenant_utf);
+                if (tenant_utf != nullptr)
+                {
+                    env->ReleaseStringUTFChars(tenant_j, tenant_utf);
+                }
                 env->ReleaseByteArrayElements(blob_j, elements, 0);
                 env.popLocalFrame();
             }

lib/pal/universal/WindowsRuntimeSystemInformationImpl.cpp

@@ -80,7 +80,15 @@ namespace PAL_NS_BEGIN {
 
         // The DeviceFamilyVersion is a decimalized form of the ULONGLONG hex form. For example:
         // 2814750430068736 = 000A000027840000 = 10.0.10116.0
-        auto versionDec = std::stoull(AnalyticsInfo::VersionInfo->DeviceFamilyVersion->Data());
+        unsigned long long versionDec = 0ull;
+        try
+        {
+            versionDec = std::stoull(AnalyticsInfo::VersionInfo->DeviceFamilyVersion->Data());
+        }
+        catch (const std::exception&)
+        {
+            versionDec = 0ull;
+        }
         if (versionDec != 0ull)
         {
             m_os_major_version = std::to_string(versionDec >> 16 * 3) + "." + std::to_string(versionDec >> 16 * 2 & 0xFFFF);