diff --git a/lib/http/HttpClient_WinInet.cpp b/lib/http/HttpClient_WinInet.cpp index eaefb2318..9c920682d 100644 --- a/lib/http/HttpClient_WinInet.cpp +++ b/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(); + } } } diff --git a/lib/http/HttpClient_WinRt.cpp b/lib/http/HttpClient_WinRt.cpp index e46e4c49c..db14aa1da 100644 --- a/lib/http/HttpClient_WinRt.cpp +++ b/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 lock(m_requestsMutex); + done = m_requests.empty(); + } + while (!done) { PAL::sleep(100); std::this_thread::yield(); + { + std::lock_guard lock(m_requestsMutex); + done = m_requests.empty(); + } } }; diff --git a/lib/jni/JniConvertors.cpp b/lib/jni/JniConvertors.cpp index 87b7cb15b..e1b0afe5a 100644 --- a/lib/jni/JniConvertors.cpp +++ b/lib/jni/JniConvertors.cpp @@ -13,6 +13,13 @@ 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) { + // GetStringUTFChars failed (e.g. OOM) and left a pending exception. Clear + // it so callers do not keep issuing JNI calls with an exception in flight. + if (env->ExceptionCheck() == JNI_TRUE) + env->ExceptionClear(); + return ""; + } std::string str(jstr_utf, jstr_utf + jstr_length); env->ReleaseStringUTFChars(jstr, jstr_utf); return str; diff --git a/lib/jni/Signals_jni.cpp b/lib/jni/Signals_jni.cpp index 59ca6f32d..ec1be2dce 100644 --- a/lib/jni/Signals_jni.cpp +++ b/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(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(timeout_ms); config.ServiceRequestConfig.RetryTimes = reinterpret_cast(retry_times); diff --git a/lib/offline/OfflineStorage_Room.cpp b/lib/offline/OfflineStorage_Room.cpp index d052e7a9d..5ea0611e0 100644 --- a/lib/offline/OfflineStorage_Room.cpp +++ b/lib/offline/OfflineStorage_Room.cpp @@ -495,7 +495,9 @@ namespace MAT_NS_BEGIN auto tenantToken_java = static_cast(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(std::max(latency_lb, std::min( @@ -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(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(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(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,13 @@ namespace MAT_NS_BEGIN auto id_j = env->GetLongField(record, id_id); auto tenant_j = static_cast(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; + // Clear/handle any pending exception from a failed string read + // (e.g. OOM) before making further JNI calls, consistent with the + // other read paths in this file. + ThrowRuntime(env, "string tenant"); auto latency = static_cast(env->GetIntField(record, latency_id)); auto persistence = static_cast(env->GetIntField(record, @@ -1359,14 +1380,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(); } diff --git a/lib/pal/universal/WindowsRuntimeSystemInformationImpl.cpp b/lib/pal/universal/WindowsRuntimeSystemInformationImpl.cpp index 2ae7e9af4..e8b1097eb 100644 --- a/lib/pal/universal/WindowsRuntimeSystemInformationImpl.cpp +++ b/lib/pal/universal/WindowsRuntimeSystemInformationImpl.cpp @@ -5,6 +5,7 @@ #include "pal/PAL.hpp" #include +#include #include "ISystemInformation.hpp" #include "pal/SystemInformationImpl.hpp" @@ -80,7 +81,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);