From 7244947036c1af35c96d6974f52a8a9e0a9e2b83 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Sun, 23 Aug 2026 23:39:25 -0500 Subject: [PATCH 1/5] Harden Android JNI resource ownership Fix normal Android JNI paths that could retain invalid pointers, leak JNI references, dispatch UUID context through the wrong overload, or publish partially initialized Privacy Guard state. Files changed: - lib/jni/LogManager_jni.cpp: validate string conversion, correct UUID context, and safely own debug callbacks. - lib/jni/JniConvertors.cpp: stop array conversion when JNI raises an exception. - lib/jni/PrivacyGuard_jni.cpp: co-own borrowed event names and fail before singleton publication. - lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogManagerProvider.java: reset listener identities after final removal. - lib/android_build/app/src/androidTest/java/com/microsoft/applications/events/maesdktest/LogManagerDDVUnitTest.java: cover listener identity reuse and release. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d7d2f27a-7339-4585-ad02-9f89ce20ef40 --- .../maesdktest/LogManagerDDVUnitTest.java | 7 + .../events/LogManagerProvider.java | 6 +- lib/jni/JniConvertors.cpp | 18 +- lib/jni/LogManager_jni.cpp | 463 ++++++++++++++---- lib/jni/PrivacyGuard_jni.cpp | 102 +++- 5 files changed, 467 insertions(+), 129 deletions(-) diff --git a/lib/android_build/app/src/androidTest/java/com/microsoft/applications/events/maesdktest/LogManagerDDVUnitTest.java b/lib/android_build/app/src/androidTest/java/com/microsoft/applications/events/maesdktest/LogManagerDDVUnitTest.java index 5f784a563..405749318 100644 --- a/lib/android_build/app/src/androidTest/java/com/microsoft/applications/events/maesdktest/LogManagerDDVUnitTest.java +++ b/lib/android_build/app/src/androidTest/java/com/microsoft/applications/events/maesdktest/LogManagerDDVUnitTest.java @@ -511,6 +511,10 @@ public void onDebugEvent(DebugEvent evt) { ListenForFilter listener = new ListenForFilter(); manager.addEventListener(DebugEventType.EVT_FILTERED, listener); + long listenerIdentity = listener.nativeIdentity; + assertThat(listenerIdentity, is(not(-1L))); + manager.addEventListener(DebugEventType.EVT_REJECTED, listener); + assertThat(listener.nativeIdentity, is(listenerIdentity)); logger.logEvent("noprops"); manager.uploadNow(); try { @@ -531,6 +535,9 @@ public void onDebugEvent(DebugEvent evt) { assertThat(listener.filteredCount, is(1L)); } manager.removeEventListener(DebugEventType.EVT_FILTERED, listener); + assertThat(listener.nativeIdentity, is(listenerIdentity)); + manager.removeEventListener(DebugEventType.EVT_REJECTED, listener); + assertThat(listener.nativeIdentity, is(-1L)); int[] everything = { DiagLevel.DIAG_LEVEL_REQUIRED.value(), DiagLevel.DIAG_LEVEL_OPTIONAL.value() }; manager.setLevelFilter(DiagLevel.DIAG_LEVEL_OPTIONAL.value(), everything); } diff --git a/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogManagerProvider.java b/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogManagerProvider.java index edd0d1771..ba5d41e74 100644 --- a/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogManagerProvider.java +++ b/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogManagerProvider.java @@ -272,11 +272,13 @@ public void addEventListener(DebugEventType eventType, DebugEventListener listen listener.nativeIdentity = nativeAddEventListener(nativeLogManager, eventType.value(), listener, listener.nativeIdentity); } - public native void nativeRemoveEventListener(long nativeLogManager, long eventType, long identity); + public native long nativeRemoveEventListener( + long nativeLogManager, long eventType, long identity, DebugEventListener listener); @Override public void removeEventListener(DebugEventType eventType, DebugEventListener listener) { - nativeRemoveEventListener(nativeLogManager, eventType.value(), listener.nativeIdentity); + listener.nativeIdentity = nativeRemoveEventListener( + nativeLogManager, eventType.value(), listener.nativeIdentity, listener); } private native boolean nativeRegisterPrivacyGuard(long nativeLogManager); diff --git a/lib/jni/JniConvertors.cpp b/lib/jni/JniConvertors.cpp index dc61a4304..9f537f21e 100644 --- a/lib/jni/JniConvertors.cpp +++ b/lib/jni/JniConvertors.cpp @@ -196,12 +196,26 @@ EventProperties GetEventProperties(JNIEnv* env, const jstring& jstrEventName, co std::vector ConvertJObjectArrayToStdStringVector(JNIEnv* env, const jobjectArray& jArrayToConvert) { std::vector stringVector; - stringVector.reserve(env->GetArrayLength(jArrayToConvert)); + auto length = env->GetArrayLength(jArrayToConvert); + if (env->ExceptionCheck()) + { + return stringVector; + } + stringVector.reserve(length); - for(int i = 0; i < env->GetArrayLength(jArrayToConvert); i++) + for(int i = 0; i < length; i++) { auto jStringValue = static_cast(env->GetObjectArrayElement(jArrayToConvert, i)); + if (env->ExceptionCheck()) + { + return stringVector; + } auto stringValue = JStringToStdString(env, jStringValue); + if (env->ExceptionCheck()) + { + env->DeleteLocalRef(jStringValue); + return stringVector; + } if(!stringValue.empty()) { stringVector.emplace_back(std::move(stringValue)); diff --git a/lib/jni/LogManager_jni.cpp b/lib/jni/LogManager_jni.cpp index ed1cfcef7..6b54ff8ec 100644 --- a/lib/jni/LogManager_jni.cpp +++ b/lib/jni/LogManager_jni.cpp @@ -399,6 +399,17 @@ namespace } } + bool TryJStringToStdString(JNIEnv* env, jstring value, std::string& result) + { + if (value == nullptr) + { + return false; + } + + result = JStringToStdString(env, value); + return !env->ExceptionCheck(); + } + /** * Smart object to manage PushLocalFrame/PopLocalFrame */ @@ -578,10 +589,12 @@ namespace continue; } auto key = static_cast(k); - auto cstringKey = env->GetStringUTFChars(key, nullptr); - rethrow(env); - std::string stringKey(cstringKey); - env->ReleaseStringUTFChars(key, cstringKey); + std::string stringKey; + if (!TryJStringToStdString(env, key, stringKey)) + { + rethrow(env); + throw std::runtime_error("Unable to convert configuration key"); + } auto value = env->CallObjectMethod(configuration, getMethod, key); rethrow(env); if (!value) @@ -634,12 +647,12 @@ namespace case ValueTypes::STRING: { auto s = static_cast(value); - auto cString = env->GetStringUTFChars( - s, - nullptr); - rethrow(env); - std::string cppString(cString); - env->ReleaseStringUTFChars(s, cString); + std::string cppString; + if (!TryJStringToStdString(env, s, cppString)) + { + rethrow(env); + throw std::runtime_error("Unable to convert string value"); + } return Variant(std::move(cppString)); } case ValueTypes::VARIANT_MAP: @@ -671,9 +684,12 @@ namespace auto jName = static_cast(env->CallObjectMethod(actual, gnMethod)); - auto cName = env->GetStringUTFChars(jName, nullptr); - std::string className(cName); - env->ReleaseStringUTFChars(jName, cName); + std::string className; + if (!TryJStringToStdString(env, jName, className)) + { + rethrow(env); + throw std::runtime_error("Unable to convert class name"); + } __android_log_print(ANDROID_LOG_ERROR, "MAE", "Unsupported class %s", @@ -869,10 +885,11 @@ Java_com_microsoft_applications_events_LogManager_nativeInitializeConfig(JNIEnv* __android_log_print(ANDROID_LOG_INFO, "MAE", "Translated map: %s", cereal.c_str()); - auto tokenUTF = env->GetStringUTFChars(tenant_token, nullptr); - rethrow(env); - std::string token(tokenUTF); - env->ReleaseStringUTFChars(tenant_token, tokenUTF); + std::string token; + if (!TryJStringToStdString(env, tenant_token, token)) + { + return 0; + } auto logger = WrapperLogManager::Initialize(token, logConfiguration); return reinterpret_cast(logger); } @@ -1012,15 +1029,15 @@ Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_na if (!mc) return 0; } - auto tokenUtf = env->GetStringUTFChars(jToken, nullptr); - std::string token{tokenUtf}; - env->ReleaseStringUTFChars(jToken, tokenUtf); - auto sourceUtf = env->GetStringUTFChars(jSource, nullptr); - std::string source{sourceUtf}; - env->ReleaseStringUTFChars(jSource, sourceUtf); - auto scopeUtf = env->GetStringUTFChars(jScope, nullptr); - std::string scope{scopeUtf}; - env->ReleaseStringUTFChars(jScope, scopeUtf); + std::string token; + std::string source; + std::string scope; + if (!TryJStringToStdString(env, jToken, token) || + !TryJStringToStdString(env, jSource, source) || + !TryJStringToStdString(env, jScope, scope)) + { + return 0; + } return reinterpret_cast(mc->manager->GetLogger( token, source, @@ -1134,9 +1151,11 @@ Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_na { return STATUS_EFAIL; } - auto profile_string = env->GetStringUTFChars(profile, nullptr); - std::string stringyProfile(profile_string); - env->ReleaseStringUTFChars(profile, profile_string); + std::string stringyProfile; + if (!TryJStringToStdString(env, profile, stringyProfile)) + { + return STATUS_EFAIL; + } return logManager->SetTransmitProfile(stringyProfile); } @@ -1152,9 +1171,11 @@ Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_na { return STATUS_EFAIL; } - auto chars = env->GetStringUTFChars(json, nullptr); - std::string cppJson(chars); - env->ReleaseStringUTFChars(json, chars); + std::string cppJson; + if (!TryJStringToStdString(env, json, cppJson)) + { + return STATUS_EFAIL; + } return logManager->LoadTransmitProfiles(cppJson); } @@ -1215,12 +1236,13 @@ Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_na { return STATUS_EFAIL; } - auto chars = env->GetStringUTFChars(name, nullptr); - std::string cppName(chars); - env->ReleaseStringUTFChars(name, chars); - chars = env->GetStringUTFChars(value, nullptr); - std::string cppValue(chars); - env->ReleaseStringUTFChars(value, chars); + std::string cppName; + std::string cppValue; + if (!TryJStringToStdString(env, name, cppName) || + !TryJStringToStdString(env, value, cppValue)) + { + return STATUS_EFAIL; + } return logManager->SetContext(cppName, cppValue, static_cast(pii_kind)); @@ -1240,9 +1262,11 @@ Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_na { return STATUS_EFAIL; } - auto chars = env->GetStringUTFChars(name, nullptr); - std::string cppName(chars); - env->ReleaseStringUTFChars(name, chars); + std::string cppName; + if (!TryJStringToStdString(env, name, cppName)) + { + return STATUS_EFAIL; + } return logManager->SetContext(cppName, value, static_cast(pii_kind)); } @@ -1261,9 +1285,11 @@ Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_na { return STATUS_EFAIL; } - auto chars = env->GetStringUTFChars(name, nullptr); - std::string cppName(chars); - env->ReleaseStringUTFChars(name, chars); + std::string cppName; + if (!TryJStringToStdString(env, name, cppName)) + { + return STATUS_EFAIL; + } return logManager->SetContext(cppName, value, static_cast(pii_kind)); } @@ -1282,9 +1308,11 @@ Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_na { return STATUS_EFAIL; } - auto chars = env->GetStringUTFChars(name, nullptr); - std::string cppName(chars); - env->ReleaseStringUTFChars(name, chars); + std::string cppName; + if (!TryJStringToStdString(env, name, cppName)) + { + return STATUS_EFAIL; + } return logManager->SetContext(cppName, value, static_cast(pii_kind)); } @@ -1303,9 +1331,11 @@ Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_na { return STATUS_EFAIL; } - auto chars = env->GetStringUTFChars(name, nullptr); - std::string cppName(chars); - env->ReleaseStringUTFChars(name, chars); + std::string cppName; + if (!TryJStringToStdString(env, name, cppName)) + { + return STATUS_EFAIL; + } return logManager->SetContext(cppName, value, static_cast(pii_kind)); } @@ -1324,9 +1354,11 @@ Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_na { return STATUS_EFAIL; } - auto chars = env->GetStringUTFChars(name, nullptr); - std::string cppName(chars); - env->ReleaseStringUTFChars(name, chars); + std::string cppName; + if (!TryJStringToStdString(env, name, cppName)) + { + return STATUS_EFAIL; + } auto dateClass = env->GetObjectClass(value); auto getTimeID = env->GetMethodID(dateClass, "getTime", "()J"); auto javaMilliseconds = env->CallLongMethod(value, getTimeID); @@ -1351,14 +1383,15 @@ Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_na { return STATUS_EFAIL; } - auto chars = env->GetStringUTFChars(name, nullptr); - std::string cppName(chars); - env->ReleaseStringUTFChars(name, chars); - chars = env->GetStringUTFChars(value, nullptr); - auto result = logManager->SetContext(cppName, value, - static_cast(pii_kind)); - env->ReleaseStringUTFChars(value, chars); - return result; + std::string cppName; + std::string cppValue; + if (!TryJStringToStdString(env, name, cppName) || + !TryJStringToStdString(env, value, cppValue)) + { + return STATUS_EFAIL; + } + return logManager->SetContext(cppName, GUID_t(cppValue.c_str()), + static_cast(pii_kind)); } extern "C" JNIEXPORT jboolean JNICALL @@ -1539,49 +1572,171 @@ namespace { struct JniDebugEventListener : DebugEventListener { + struct Registration + { + jlong logManager; + DebugEventType eventType; + }; + JavaVM* javaVm; - jobject javaListener; + jobject javaListener = nullptr; + jclass eventClass = nullptr; + jmethodID eventConstructor = nullptr; + jmethodID listenerMethod = nullptr; + std::vector registrations; JniDebugEventListener() = delete; - JniDebugEventListener(JavaVM* _javaVm, jobject _javaListener) : - javaVm(_javaVm) + JniDebugEventListener(JNIEnv* env, JavaVM* vm, jobject listener) : + javaVm(vm) { - JNIEnv* env = nullptr; - _javaVm->AttachCurrentThread(&env, nullptr); - javaListener = env->NewGlobalRef(_javaListener); + auto localEventClass = + env->FindClass("com/microsoft/applications/events/DebugEvent"); + rethrow(env); + eventConstructor = env->GetMethodID( + localEventClass, "", "(JJJJJLjava/lang/Object;J)V"); + rethrow(env); + + auto localListenerClass = env->GetObjectClass(listener); + rethrow(env); + listenerMethod = env->GetMethodID( + localListenerClass, + "onDebugEvent", + "(Lcom/microsoft/applications/events/DebugEvent;)V"); + rethrow(env); + + eventClass = static_cast(env->NewGlobalRef(localEventClass)); + rethrow(env); + javaListener = env->NewGlobalRef(listener); + if (javaListener == nullptr) + { + env->DeleteGlobalRef(eventClass); + eventClass = nullptr; + rethrow(env); + throw std::runtime_error("Unable to retain debug event listener"); + } + + env->DeleteLocalRef(localListenerClass); + env->DeleteLocalRef(localEventClass); } - ~JniDebugEventListener() + ~JniDebugEventListener() override { - JNIEnv* env = nullptr; - javaVm->AttachCurrentThread(&env, nullptr); + bool detach = false; + auto env = GetEnv(detach); + if (env == nullptr) + { + return; + } + env->DeleteGlobalRef(javaListener); + env->DeleteGlobalRef(eventClass); + if (detach) + { + javaVm->DetachCurrentThread(); + } } void OnDebugEvent(DebugEvent& evt) override { + bool detach = false; + auto env = GetEnv(detach); + if (env == nullptr) + { + return; + } + + if (env->PushLocalFrame(1) == JNI_OK) + { + auto eventLocal = env->NewObject( + eventClass, + eventConstructor, + static_cast(evt.seq), + static_cast(evt.ts), + static_cast(evt.type), + static_cast(evt.param1), + static_cast(evt.param2), + static_cast(nullptr), + static_cast(evt.size)); + if (eventLocal != nullptr && !env->ExceptionCheck()) + { + env->CallVoidMethod(javaListener, listenerMethod, eventLocal); + } + env->PopLocalFrame(nullptr); + } + + if (env->ExceptionCheck()) + { + env->ExceptionDescribe(); + env->ExceptionClear(); + } + if (detach) + { + javaVm->DetachCurrentThread(); + } + } + + bool IsSameListener(JNIEnv* env, jobject listener) const + { + return env->IsSameObject(javaListener, listener) == JNI_TRUE; + } + + bool AddRegistration(jlong logManager, DebugEventType eventType) + { + auto existing = std::find_if( + registrations.begin(), + registrations.end(), + [logManager, eventType](const Registration& registration) { + return registration.logManager == logManager && + registration.eventType == eventType; + }); + if (existing != registrations.end()) + { + return false; + } + + registrations.push_back({logManager, eventType}); + return true; + } + + bool RemoveRegistration(jlong logManager, DebugEventType eventType) + { + auto existing = std::find_if( + registrations.begin(), + registrations.end(), + [logManager, eventType](const Registration& registration) { + return registration.logManager == logManager && + registration.eventType == eventType; + }); + if (existing == registrations.end()) + { + return false; + } + + registrations.erase(existing); + return true; + } + + private: + JNIEnv* GetEnv(bool& detach) const + { + detach = false; JNIEnv* env = nullptr; - javaVm->AttachCurrentThread(&env, nullptr); - auto eventClassId = - env->FindClass("com/microsoft/applications/events/DebugEvent"); - auto constructorId = env->GetMethodID(eventClassId, "", - "(JJJJJLjava/lang/Object;J)V"); - jobject eventLocal; - eventLocal = env->NewObject(eventClassId, - constructorId, - static_cast(evt.seq), - static_cast(evt.ts), - static_cast(evt.type), - static_cast(evt.param1), - static_cast(evt.param2), - static_cast(nullptr), - static_cast(evt.size)); - auto classId = env->GetObjectClass(javaListener); - auto methodId = env->GetMethodID(classId, - "onDebugEvent", - "(Lcom/microsoft/applications/events/DebugEvent;)V"); - env->CallVoidMethod(javaListener, methodId, eventLocal); + auto status = javaVm->GetEnv(reinterpret_cast(&env), JNI_VERSION_1_6); + if (status == JNI_EDETACHED) + { + if (javaVm->AttachCurrentThread(&env, nullptr) != JNI_OK) + { + return nullptr; + } + detach = true; + } + else if (status != JNI_OK) + { + return nullptr; + } + + return env; } }; @@ -1598,34 +1753,130 @@ Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_na jobject listener, jlong current_identity) { - JavaVM* vm; - env->GetJavaVM(&vm); - std::unique_ptr callback = std::make_unique(vm, listener); auto logManager = getLogManager(native_log_manager); - logManager->AddEventListener(static_cast(event_type), *callback); - if (current_identity >= 0) { - return current_identity; + if (logManager == nullptr || listener == nullptr) + { + return -1; + } + + auto eventType = static_cast(event_type); + JniDebugEventListener* callback = nullptr; + jlong identity = -1; + { + std::lock_guard lock(listeners_mutex); + if (current_identity >= 0 && + current_identity < static_cast(listeners.size()) && + listeners[current_identity] && + listeners[current_identity]->IsSameListener(env, listener)) + { + callback = listeners[current_identity].get(); + identity = current_identity; + } } - std::lock_guard l(listeners_mutex); - listeners.emplace_back(std::move(callback)); - return listeners.size(); + + if (!callback) + { + JavaVM* vm = nullptr; + if (env->GetJavaVM(&vm) != JNI_OK) + { + return -1; + } + + try + { + auto newCallback = std::make_unique(env, vm, listener); + callback = newCallback.get(); + + std::lock_guard lock(listeners_mutex); + auto available = std::find(listeners.begin(), listeners.end(), nullptr); + if (available == listeners.end()) + { + identity = static_cast(listeners.size()); + listeners.emplace_back(std::move(newCallback)); + } + else + { + identity = static_cast(std::distance(listeners.begin(), available)); + *available = std::move(newCallback); + } + } + catch (const std::exception& e) + { + if (!env->ExceptionCheck()) + { + auto exceptionClass = env->FindClass("java/lang/RuntimeException"); + if (exceptionClass != nullptr) + { + env->ThrowNew(exceptionClass, e.what()); + env->DeleteLocalRef(exceptionClass); + } + } + return -1; + } + } + + { + std::lock_guard lock(listeners_mutex); + if (!callback->AddRegistration(native_log_manager, eventType)) + { + return identity; + } + } + logManager->AddEventListener(eventType, *callback); + return identity; } extern "C" -JNIEXPORT void JNICALL +JNIEXPORT jlong JNICALL Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_nativeRemoveEventListener( JNIEnv *env, jobject thiz, jlong native_log_manager, jlong eventType, - jlong identity) { - std::lock_guard l(listeners_mutex); - if (identity < 0 || identity >= static_cast(jniManagers.size()) || !listeners[identity]) - { - return; + jlong identity, + jobject listener) { + JniDebugEventListener* callback = nullptr; + auto newIdentity = identity; + { + std::lock_guard lock(listeners_mutex); + if (identity < 0 || + identity >= static_cast(listeners.size()) || + !listeners[identity] || + !listeners[identity]->IsSameListener(env, listener)) + { + return identity; + } + + callback = listeners[identity].get(); + if (!callback->RemoveRegistration( + native_log_manager, + static_cast(eventType))) + { + return identity; + } + if (callback->registrations.empty()) + { + newIdentity = -1; + } } + auto logManager = getLogManager(native_log_manager); - logManager->RemoveEventListener(static_cast(eventType), *listeners[identity]); + if (logManager != nullptr) + { + logManager->RemoveEventListener( + static_cast(eventType), + *callback); + } + if (newIdentity < 0) + { + std::lock_guard lock(listeners_mutex); + if (listeners[identity].get() == callback && + callback->registrations.empty()) + { + listeners[identity].reset(); + } + } + return newIdentity; } extern "C" diff --git a/lib/jni/PrivacyGuard_jni.cpp b/lib/jni/PrivacyGuard_jni.cpp index 8fd23867a..a9f923081 100644 --- a/lib/jni/PrivacyGuard_jni.cpp +++ b/lib/jni/PrivacyGuard_jni.cpp @@ -22,21 +22,54 @@ CommonDataContext GenerateCommonDataContextObject(JNIEnv* env, CommonDataContext cdc; if(domainName != nullptr) { cdc.DomainName = JStringToStdString(env, domainName); + if (env->ExceptionCheck()) { + return cdc; + } } if(machineName != nullptr) { cdc.MachineName = JStringToStdString(env, machineName); + if (env->ExceptionCheck()) { + return cdc; + } } cdc.UserNames = ConvertJObjectArrayToStdStringVector(env, userNames); + if (env->ExceptionCheck()) { + return cdc; + } cdc.UserAliases = ConvertJObjectArrayToStdStringVector(env, userAliases); + if (env->ExceptionCheck()) { + return cdc; + } cdc.IpAddresses = ConvertJObjectArrayToStdStringVector(env, ipAddresses); + if (env->ExceptionCheck()) { + return cdc; + } cdc.LanguageIdentifiers = ConvertJObjectArrayToStdStringVector(env, languageIdentifiers); + if (env->ExceptionCheck()) { + return cdc; + } cdc.MachineIds = ConvertJObjectArrayToStdStringVector(env, machineIds); + if (env->ExceptionCheck()) { + return cdc; + } cdc.OutOfScopeIdentifiers = ConvertJObjectArrayToStdStringVector(env, outOfScopeIdentifiers); return cdc; } -std::shared_ptr spPrivacyGuard; +namespace +{ + // PrivacyGuard borrows its configured event names, so keep their storage with the guard. + struct PrivacyGuardState + { + std::string notificationEventName; + std::string semanticContextEventName; + std::string summaryEventName; + std::unique_ptr privacyGuard; + }; + + std::shared_ptr spPrivacyGuard; +} std::shared_ptr PrivacyGuardHelper::GetPrivacyGuardPtr() noexcept { @@ -62,16 +95,29 @@ Java_com_microsoft_applications_events_PrivacyGuard_nativeInitializePrivacyGuard InitializationConfiguration config( reinterpret_cast(iLoggerNativePtr), CommonDataContext{}); + auto state = std::make_shared(); if (NotificationEventName != nullptr) { - config.NotificationEventName = JStringToStdString(env, NotificationEventName).c_str(); + state->notificationEventName = JStringToStdString(env, NotificationEventName); + if (env->ExceptionCheck()) { + return false; + } + config.NotificationEventName = state->notificationEventName.c_str(); } if (SemanticContextEventName != nullptr) { - config.SemanticContextNotificationEventName = JStringToStdString(env, SemanticContextEventName).c_str(); + state->semanticContextEventName = JStringToStdString(env, SemanticContextEventName); + if (env->ExceptionCheck()) { + return false; + } + config.SemanticContextNotificationEventName = state->semanticContextEventName.c_str(); } if (SummaryEventName != nullptr) { - config.SummaryEventName = JStringToStdString(env, SummaryEventName).c_str(); + state->summaryEventName = JStringToStdString(env, SummaryEventName); + if (env->ExceptionCheck()) { + return false; + } + config.SummaryEventName = state->summaryEventName.c_str(); } config.UseEventFieldPrefix = static_cast(UseEventFieldPrefix); @@ -79,7 +125,8 @@ Java_com_microsoft_applications_events_PrivacyGuard_nativeInitializePrivacyGuard config.DisableAdvancedScans = static_cast(DisableAdvancedScans); config.StampEventIKeyForConcerns = static_cast(StampEventIKeyForConcerns); - spPrivacyGuard = std::make_shared(config); + state->privacyGuard = std::make_unique(config); + spPrivacyGuard = std::shared_ptr(state, state->privacyGuard.get()); return true; } @@ -107,28 +154,45 @@ Java_com_microsoft_applications_events_PrivacyGuard_nativeInitializePrivacyGuard return false; } + auto commonDataContext = GenerateCommonDataContextObject(env, + domainName, + machineName, + userNames, + userAliases, + ipAddresses, + languageIdentifiers, + machineIds, + outOfScopeIdentifiers); + if (env->ExceptionCheck()) { + return false; + } InitializationConfiguration config( reinterpret_cast(iLoggerNativePtr), - GenerateCommonDataContextObject(env, - domainName, - machineName, - userNames, - userAliases, - ipAddresses, - languageIdentifiers, - machineIds, - outOfScopeIdentifiers)); + commonDataContext); + auto state = std::make_shared(); if (NotificationEventName != NULL) { - config.NotificationEventName = JStringToStdString(env, NotificationEventName).c_str(); + state->notificationEventName = JStringToStdString(env, NotificationEventName); + if (env->ExceptionCheck()) { + return false; + } + config.NotificationEventName = state->notificationEventName.c_str(); } if (SemanticContextEventName != NULL) { - config.SemanticContextNotificationEventName = JStringToStdString(env, SemanticContextEventName).c_str(); + state->semanticContextEventName = JStringToStdString(env, SemanticContextEventName); + if (env->ExceptionCheck()) { + return false; + } + config.SemanticContextNotificationEventName = state->semanticContextEventName.c_str(); } if (SummaryEventName != NULL) { - config.SummaryEventName = JStringToStdString(env, SummaryEventName).c_str(); + state->summaryEventName = JStringToStdString(env, SummaryEventName); + if (env->ExceptionCheck()) { + return false; + } + config.SummaryEventName = state->summaryEventName.c_str(); } config.UseEventFieldPrefix = static_cast(UseEventFieldPrefix); @@ -136,7 +200,8 @@ Java_com_microsoft_applications_events_PrivacyGuard_nativeInitializePrivacyGuard config.DisableAdvancedScans = static_cast(DisableAdvancedScans); config.StampEventIKeyForConcerns = static_cast(StampEventIKeyForConcerns); - spPrivacyGuard = std::make_shared(config); + state->privacyGuard = std::make_unique(config); + spPrivacyGuard = std::shared_ptr(state, state->privacyGuard.get()); return true; } @@ -221,4 +286,3 @@ JNIEXPORT jboolean JNICALL Java_com_microsoft_applications_events_PrivacyGuard_isInitialized(const JNIEnv *env, jclass/* this */){ return spPrivacyGuard != nullptr; } - From aee4777a8a9bc1038cb29822b83e89eabcf745cb Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 24 Aug 2026 13:04:04 -0500 Subject: [PATCH 2/5] Make JNI listener teardown race-safe Pin JNI callbacks across reentrant dispatch, make add/remove registration transitions transactional, and atomically snapshot Privacy Guard state so teardown cannot race active users or commit partial JNI conversions. Files changed: - lib/callbacks/DebugSource.cpp: snapshot dispatch listeners and track pending callback lifetimes. - lib/callbacks/DebugSourceInternal.hpp: expose internal pending-listener cleanup hooks. - lib/jni/LogManager_jni.cpp: serialize listener state transitions without lock inversion and retain callbacks through dispatch. - lib/jni/PrivacyGuard_jni.cpp: publish atomic shared snapshots and reject partial conversion results. - tests/unittests/DebugEventSourceTests.cpp: cover reentrant removal during dispatch. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: de6b1234-574e-4b12-9f45-88ff809818ab --- lib/callbacks/DebugSource.cpp | 81 +++++- lib/callbacks/DebugSourceInternal.hpp | 18 ++ lib/jni/LogManager_jni.cpp | 319 ++++++++++++++++------ lib/jni/PrivacyGuard_jni.cpp | 75 +++-- tests/unittests/DebugEventSourceTests.cpp | 22 +- 5 files changed, 404 insertions(+), 111 deletions(-) create mode 100644 lib/callbacks/DebugSourceInternal.hpp diff --git a/lib/callbacks/DebugSource.cpp b/lib/callbacks/DebugSource.cpp index 8662b9088..cc70009d3 100644 --- a/lib/callbacks/DebugSource.cpp +++ b/lib/callbacks/DebugSource.cpp @@ -3,14 +3,88 @@ // SPDX-License-Identifier: Apache-2.0 // #include "mat/config.h" +#include "callbacks/DebugSourceInternal.hpp" #include "DebugEvents.hpp" #include "utils/Utils.hpp" #include "pal/PAL.hpp" #include +#include namespace MAT_NS_BEGIN { + namespace + { + thread_local std::vector pendingListeners; + std::atomic + pendingReleaseCallback{nullptr}; + + class PendingListenersScope + { + public: + explicit PendingListenersScope(const std::vector& listeners) : + remaining(listeners) + { + pendingListeners.insert( + pendingListeners.end(), + listeners.begin(), + listeners.end()); + } + + ~PendingListenersScope() + { + for (auto listener : remaining) + { + RemovePending(listener); + auto callback = pendingReleaseCallback.load(); + if (callback != nullptr) + { + callback(listener); + } + } + } + + void BeginCallback(DebugEventListener* listener) + { + auto current = std::find(remaining.begin(), remaining.end(), listener); + if (current != remaining.end()) + { + remaining.erase(current); + } + RemovePending(listener); + } + + private: + static void RemovePending(DebugEventListener* listener) + { + auto pending = std::find( + pendingListeners.rbegin(), + pendingListeners.rend(), + listener); + if (pending != pendingListeners.rend()) + { + pendingListeners.erase(std::next(pending).base()); + } + } + + std::vector remaining; + }; + } + + bool IsDebugEventListenerPending(const DebugEventListener* listener) noexcept + { + return std::find( + pendingListeners.begin(), + pendingListeners.end(), + listener) != pendingListeners.end(); + } + + void SetDebugEventListenerPendingReleaseCallback( + DebugEventListenerPendingReleaseCallback callback) noexcept + { + pendingReleaseCallback.store(callback); + } + /// Add event listener for specific debug event type. void DebugEventSource::AddEventListener(DebugEventType type, DebugEventListener &listener) { @@ -45,8 +119,10 @@ namespace MAT_NS_BEGIN { if (listeners.size()) { // Events filter handlers list - auto &v = listeners[evt.type]; - for (auto listener : v) { + auto eventListeners = listeners[evt.type]; + PendingListenersScope pendingScope(eventListeners); + for (auto listener : eventListeners) { + pendingScope.BeginCallback(listener); listener->OnDebugEvent(evt); dispatched = true; } @@ -85,4 +161,3 @@ namespace MAT_NS_BEGIN { } } MAT_NS_END - diff --git a/lib/callbacks/DebugSourceInternal.hpp b/lib/callbacks/DebugSourceInternal.hpp new file mode 100644 index 000000000..5adfe75bd --- /dev/null +++ b/lib/callbacks/DebugSourceInternal.hpp @@ -0,0 +1,18 @@ +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +#pragma once + +#include "DebugEvents.hpp" + +namespace MAT_NS_BEGIN +{ + using DebugEventListenerPendingReleaseCallback = + void (*)(DebugEventListener*); + + bool IsDebugEventListenerPending(const DebugEventListener* listener) noexcept; + void SetDebugEventListenerPendingReleaseCallback( + DebugEventListenerPendingReleaseCallback callback) noexcept; +} +MAT_NS_END diff --git a/lib/jni/LogManager_jni.cpp b/lib/jni/LogManager_jni.cpp index 6b54ff8ec..d72ebb4b0 100644 --- a/lib/jni/LogManager_jni.cpp +++ b/lib/jni/LogManager_jni.cpp @@ -26,6 +26,7 @@ #endif #include +#include "callbacks/DebugSourceInternal.hpp" #include "JniConvertors.hpp" #include "LogManagerBase.hpp" #include "WrapperLogManager.hpp" @@ -1570,12 +1571,27 @@ Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_na namespace { - struct JniDebugEventListener : DebugEventListener + struct JniDebugEventListener; + void ReleaseUnregisteredListener( + const std::shared_ptr& listener); + + struct JniDebugEventListener : + DebugEventListener, + std::enable_shared_from_this { struct Registration { + enum class State + { + Adding, + Active, + Cancelled, + Removing + }; + jlong logManager; DebugEventType eventType; + State state; }; JavaVM* javaVm; @@ -1639,6 +1655,8 @@ namespace void OnDebugEvent(DebugEvent& evt) override { + auto keepAlive = shared_from_this(); + ReleaseUnregisteredListener(keepAlive); bool detach = false; auto env = GetEnv(detach); if (env == nullptr) @@ -1672,7 +1690,7 @@ namespace } if (detach) { - javaVm->DetachCurrentThread(); + keepAlive->javaVm->DetachCurrentThread(); } } @@ -1681,40 +1699,28 @@ namespace return env->IsSameObject(javaListener, listener) == JNI_TRUE; } - bool AddRegistration(jlong logManager, DebugEventType eventType) + std::vector::iterator FindRegistration( + jlong logManager, + DebugEventType eventType) { - auto existing = std::find_if( + return std::find_if( registrations.begin(), registrations.end(), [logManager, eventType](const Registration& registration) { return registration.logManager == logManager && registration.eventType == eventType; }); - if (existing != registrations.end()) - { - return false; - } - - registrations.push_back({logManager, eventType}); - return true; } - bool RemoveRegistration(jlong logManager, DebugEventType eventType) + bool HasLiveRegistrations() const { - auto existing = std::find_if( + return std::any_of( registrations.begin(), registrations.end(), - [logManager, eventType](const Registration& registration) { - return registration.logManager == logManager && - registration.eventType == eventType; + [](const Registration& registration) { + return registration.state == Registration::State::Adding || + registration.state == Registration::State::Active; }); - if (existing == registrations.end()) - { - return false; - } - - registrations.erase(existing); - return true; } private: @@ -1740,8 +1746,52 @@ namespace } }; - static std::vector> listeners; + static std::vector> listeners; static std::mutex listeners_mutex; + + void ReleaseUnregisteredListener( + const std::shared_ptr& listener) + { + std::lock_guard lock(listeners_mutex); + if (!listener->registrations.empty() || + IsDebugEventListenerPending(listener.get())) + { + return; + } + + auto existing = std::find(listeners.begin(), listeners.end(), listener); + if (existing != listeners.end()) + { + existing->reset(); + } + } + + void ReleasePendingJniDebugEventListener( + DebugEventListener* listener) noexcept + { + std::shared_ptr callback; + { + std::lock_guard lock(listeners_mutex); + auto existing = std::find_if( + listeners.begin(), + listeners.end(), + [listener](const std::shared_ptr& value) { + return value.get() == listener; + }); + if (existing == listeners.end() || (*existing)->HasLiveRegistrations()) + { + return; + } + callback = *existing; + } + ReleaseUnregisteredListener(callback); + } + + const bool pendingReleaseCallbackRegistered = [] { + SetDebugEventListenerPendingReleaseCallback( + ReleasePendingJniDebugEventListener); + return true; + }(); } // anonymous namespace extern "C" JNIEXPORT jlong JNICALL @@ -1759,71 +1809,158 @@ Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_na return -1; } - auto eventType = static_cast(event_type); - JniDebugEventListener* callback = nullptr; - jlong identity = -1; + JavaVM* vm = nullptr; + if (env->GetJavaVM(&vm) != JNI_OK) { - std::lock_guard lock(listeners_mutex); - if (current_identity >= 0 && - current_identity < static_cast(listeners.size()) && - listeners[current_identity] && - listeners[current_identity]->IsSameListener(env, listener)) - { - callback = listeners[current_identity].get(); - identity = current_identity; - } + return -1; } - if (!callback) + try { - JavaVM* vm = nullptr; - if (env->GetJavaVM(&vm) != JNI_OK) + auto eventType = static_cast(event_type); + std::shared_ptr callback; + jlong identity = -1; { - return -1; + std::lock_guard lock(listeners_mutex); + if (current_identity >= 0 && + current_identity < static_cast(listeners.size()) && + listeners[current_identity] && + listeners[current_identity]->IsSameListener(env, listener)) + { + callback = listeners[current_identity]; + identity = current_identity; + } + + if (!callback) + { + auto existing = std::find_if( + listeners.begin(), + listeners.end(), + [env, listener](const std::shared_ptr& value) { + return value && value->IsSameListener(env, listener); + }); + if (existing != listeners.end()) + { + callback = *existing; + identity = static_cast( + std::distance(listeners.begin(), existing)); + } + } + + if (!callback) + { + callback = std::make_shared(env, vm, listener); + callback->registrations.push_back( + {native_log_manager, eventType, JniDebugEventListener::Registration::State::Adding}); + + auto available = std::find(listeners.begin(), listeners.end(), nullptr); + if (available == listeners.end()) + { + identity = static_cast(listeners.size()); + listeners.emplace_back(callback); + } + else + { + identity = static_cast(std::distance(listeners.begin(), available)); + *available = callback; + } + } + else if (callback->FindRegistration(native_log_manager, eventType) != + callback->registrations.end()) + { + return identity; + } + else + { + callback->registrations.push_back( + {native_log_manager, eventType, JniDebugEventListener::Registration::State::Adding}); + } } try { - auto newCallback = std::make_unique(env, vm, listener); - callback = newCallback.get(); + logManager->AddEventListener(eventType, *callback); + } + catch (...) + { + std::lock_guard lock(listeners_mutex); + auto registration = + callback->FindRegistration(native_log_manager, eventType); + if (registration != callback->registrations.end() && + (registration->state == + JniDebugEventListener::Registration::State::Adding || + registration->state == + JniDebugEventListener::Registration::State::Cancelled)) + { + callback->registrations.erase(registration); + } + if (callback->registrations.empty() && + !IsDebugEventListenerPending(callback.get())) + { + auto existing = std::find(listeners.begin(), listeners.end(), callback); + if (existing != listeners.end()) + { + existing->reset(); + } + } + throw; + } + bool removeCancelledRegistration = false; + { std::lock_guard lock(listeners_mutex); - auto available = std::find(listeners.begin(), listeners.end(), nullptr); - if (available == listeners.end()) + auto registration = + callback->FindRegistration(native_log_manager, eventType); + if (registration == callback->registrations.end()) { - identity = static_cast(listeners.size()); - listeners.emplace_back(std::move(newCallback)); + removeCancelledRegistration = true; + } + else if (registration->state == + JniDebugEventListener::Registration::State::Cancelled) + { + removeCancelledRegistration = true; } else { - identity = static_cast(std::distance(listeners.begin(), available)); - *available = std::move(newCallback); + registration->state = + JniDebugEventListener::Registration::State::Active; } } - catch (const std::exception& e) + + if (removeCancelledRegistration) { - if (!env->ExceptionCheck()) + logManager->RemoveEventListener(eventType, *callback); + std::lock_guard lock(listeners_mutex); + auto registration = + callback->FindRegistration(native_log_manager, eventType); + if (registration != callback->registrations.end() && + registration->state == + JniDebugEventListener::Registration::State::Cancelled) { - auto exceptionClass = env->FindClass("java/lang/RuntimeException"); - if (exceptionClass != nullptr) - { - env->ThrowNew(exceptionClass, e.what()); - env->DeleteLocalRef(exceptionClass); - } + callback->registrations.erase(registration); + } + if (callback->registrations.empty() && + !IsDebugEventListenerPending(callback.get())) + { + listeners[identity].reset(); } - return -1; } + std::lock_guard lock(listeners_mutex); + return callback->HasLiveRegistrations() ? identity : -1; } - + catch (const std::exception& e) { - std::lock_guard lock(listeners_mutex); - if (!callback->AddRegistration(native_log_manager, eventType)) + if (!env->ExceptionCheck()) { - return identity; + auto exceptionClass = env->FindClass("java/lang/RuntimeException"); + if (exceptionClass != nullptr) + { + env->ThrowNew(exceptionClass, e.what()); + env->DeleteLocalRef(exceptionClass); + } } + return -1; } - logManager->AddEventListener(eventType, *callback); - return identity; } extern "C" @@ -1835,8 +1972,9 @@ Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_na jlong eventType, jlong identity, jobject listener) { - JniDebugEventListener* callback = nullptr; - auto newIdentity = identity; + auto logManager = getLogManager(native_log_manager); + std::shared_ptr callback; + auto event = static_cast(eventType); { std::lock_guard lock(listeners_mutex); if (identity < 0 || @@ -1847,36 +1985,49 @@ Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_na return identity; } - callback = listeners[identity].get(); - if (!callback->RemoveRegistration( - native_log_manager, - static_cast(eventType))) + callback = listeners[identity]; + auto registration = + callback->FindRegistration(native_log_manager, event); + if (registration == callback->registrations.end()) { - return identity; + return callback->HasLiveRegistrations() ? identity : -1; } - if (callback->registrations.empty()) + if (registration->state == + JniDebugEventListener::Registration::State::Adding) { - newIdentity = -1; + registration->state = + JniDebugEventListener::Registration::State::Cancelled; + return callback->HasLiveRegistrations() ? identity : -1; } + if (registration->state != + JniDebugEventListener::Registration::State::Active) + { + return callback->HasLiveRegistrations() ? identity : -1; + } + registration->state = + JniDebugEventListener::Registration::State::Removing; } - auto logManager = getLogManager(native_log_manager); if (logManager != nullptr) { - logManager->RemoveEventListener( - static_cast(eventType), - *callback); + logManager->RemoveEventListener(event, *callback); } - if (newIdentity < 0) + + std::lock_guard lock(listeners_mutex); + auto registration = callback->FindRegistration(native_log_manager, event); + if (registration != callback->registrations.end() && + registration->state == + JniDebugEventListener::Registration::State::Removing) { - std::lock_guard lock(listeners_mutex); - if (listeners[identity].get() == callback && - callback->registrations.empty()) - { - listeners[identity].reset(); - } + callback->registrations.erase(registration); + } + if (listeners[identity] == callback && + callback->registrations.empty() && + !IsDebugEventListenerPending(callback.get())) + { + listeners[identity].reset(); } - return newIdentity; + return callback->HasLiveRegistrations() ? identity : -1; } extern "C" diff --git a/lib/jni/PrivacyGuard_jni.cpp b/lib/jni/PrivacyGuard_jni.cpp index a9f923081..1cbe9e96c 100644 --- a/lib/jni/PrivacyGuard_jni.cpp +++ b/lib/jni/PrivacyGuard_jni.cpp @@ -7,6 +7,10 @@ #include "modules/privacyguard/PrivacyGuard.hpp" #include "PrivacyGuardHelper.hpp" +#include +#include +#include + using namespace MAT; CommonDataContext GenerateCommonDataContextObject(JNIEnv* env, @@ -69,11 +73,12 @@ namespace }; std::shared_ptr spPrivacyGuard; + std::mutex privacyGuardMutex; } std::shared_ptr PrivacyGuardHelper::GetPrivacyGuardPtr() noexcept { - return spPrivacyGuard; + return std::atomic_load(&spPrivacyGuard); } extern "C" @@ -88,7 +93,8 @@ Java_com_microsoft_applications_events_PrivacyGuard_nativeInitializePrivacyGuard jboolean ScanForUrls, jboolean DisableAdvancedScans, jboolean StampEventIKeyForConcerns) { - if (spPrivacyGuard != nullptr) { + std::lock_guard lock(privacyGuardMutex); + if (std::atomic_load(&spPrivacyGuard) != nullptr) { return false; } @@ -126,7 +132,9 @@ Java_com_microsoft_applications_events_PrivacyGuard_nativeInitializePrivacyGuard config.StampEventIKeyForConcerns = static_cast(StampEventIKeyForConcerns); state->privacyGuard = std::make_unique(config); - spPrivacyGuard = std::shared_ptr(state, state->privacyGuard.get()); + std::atomic_store( + &spPrivacyGuard, + std::shared_ptr(state, state->privacyGuard.get())); return true; } @@ -150,7 +158,8 @@ Java_com_microsoft_applications_events_PrivacyGuard_nativeInitializePrivacyGuard jobjectArray languageIdentifiers, jobjectArray machineIds, jobjectArray outOfScopeIdentifiers) { - if (spPrivacyGuard != nullptr) { + std::lock_guard lock(privacyGuardMutex); + if (std::atomic_load(&spPrivacyGuard) != nullptr) { return false; } @@ -201,7 +210,9 @@ Java_com_microsoft_applications_events_PrivacyGuard_nativeInitializePrivacyGuard config.StampEventIKeyForConcerns = static_cast(StampEventIKeyForConcerns); state->privacyGuard = std::make_unique(config); - spPrivacyGuard = std::shared_ptr(state, state->privacyGuard.get()); + std::atomic_store( + &spPrivacyGuard, + std::shared_ptr(state, state->privacyGuard.get())); return true; } @@ -209,12 +220,13 @@ extern "C" JNIEXPORT jboolean JNICALL Java_com_microsoft_applications_events_PrivacyGuard_uninitialize(const JNIEnv *env, jclass /*this*/) { - if(spPrivacyGuard == nullptr) + std::lock_guard lock(privacyGuardMutex); + if (std::atomic_load(&spPrivacyGuard) == nullptr) { return false; } - spPrivacyGuard.reset(); + std::atomic_store(&spPrivacyGuard, std::shared_ptr{}); return true; } @@ -222,17 +234,19 @@ Java_com_microsoft_applications_events_PrivacyGuard_uninitialize(const JNIEnv *e extern "C" JNIEXPORT jboolean JNICALL Java_com_microsoft_applications_events_PrivacyGuard_setEnabled(const JNIEnv *env, jclass /*this*/, jboolean isEnabled) { - if (spPrivacyGuard == nullptr) { + auto privacyGuard = PrivacyGuardHelper::GetPrivacyGuardPtr(); + if (privacyGuard == nullptr) { return false; } - spPrivacyGuard->SetEnabled(static_cast(isEnabled)); + privacyGuard->SetEnabled(static_cast(isEnabled)); return true; } extern "C" JNIEXPORT jboolean JNICALL Java_com_microsoft_applications_events_PrivacyGuard_isEnabled(const JNIEnv *env, jclass /*this*/) { - return spPrivacyGuard != nullptr && spPrivacyGuard->IsEnabled(); + auto privacyGuard = PrivacyGuardHelper::GetPrivacyGuardPtr(); + return privacyGuard != nullptr && privacyGuard->IsEnabled(); } extern "C" @@ -247,20 +261,25 @@ Java_com_microsoft_applications_events_PrivacyGuard_nativeAppendCommonDataContex jobjectArray languageIdentifiers, jobjectArray machineIds, jobjectArray outOfScopeIdentifiers) { - if (spPrivacyGuard == nullptr) { + auto privacyGuard = PrivacyGuardHelper::GetPrivacyGuardPtr(); + if (privacyGuard == nullptr) { return false; } - spPrivacyGuard->AppendCommonDataContext(GenerateCommonDataContextObject(env, - domainName, - machineName, - userNames, - userAliases, - ipAddresses, - languageIdentifiers, - machineIds, - outOfScopeIdentifiers)); + auto commonDataContext = GenerateCommonDataContextObject(env, + domainName, + machineName, + userNames, + userAliases, + ipAddresses, + languageIdentifiers, + machineIds, + outOfScopeIdentifiers); + if (env->ExceptionCheck()) { + return false; + } + privacyGuard->AppendCommonDataContext(commonDataContext); return true; } @@ -271,18 +290,28 @@ Java_com_microsoft_applications_events_PrivacyGuard_nativeAddIgnoredConcern(JNIE jstring eventName, jstring fieldName, jint dataConcern) { - if (spPrivacyGuard == nullptr) { + auto privacyGuard = PrivacyGuardHelper::GetPrivacyGuardPtr(); + if (privacyGuard == nullptr) { return; } auto eventNameStr = JStringToStdString(env, eventName); + if (env->ExceptionCheck()) { + return; + } auto fieldNameStr = JStringToStdString(env, fieldName); + if (env->ExceptionCheck()) { + return; + } auto dataConcernInt = static_cast(dataConcern); - spPrivacyGuard->AddIgnoredConcern(eventNameStr, fieldNameStr, static_cast(dataConcernInt)); + privacyGuard->AddIgnoredConcern( + eventNameStr, + fieldNameStr, + static_cast(dataConcernInt)); } extern "C" JNIEXPORT jboolean JNICALL Java_com_microsoft_applications_events_PrivacyGuard_isInitialized(const JNIEnv *env, jclass/* this */){ - return spPrivacyGuard != nullptr; + return PrivacyGuardHelper::GetPrivacyGuardPtr() != nullptr; } diff --git a/tests/unittests/DebugEventSourceTests.cpp b/tests/unittests/DebugEventSourceTests.cpp index 5797cd83e..cf725d6db 100644 --- a/tests/unittests/DebugEventSourceTests.cpp +++ b/tests/unittests/DebugEventSourceTests.cpp @@ -188,6 +188,27 @@ TEST(DebugEventSourceTests, DispatchEvent_TwoEventsOneSameAsListenerType_Listene ASSERT_EQ(countOfEventsSeen, uint64_t { 1 }); } +TEST(DebugEventSourceTests, DispatchEvent_ListenerRemovesLaterListener_SnapshotRemainsValid) +{ + TestDebugEventSource source; + TestDebugEventListener removingListener; + TestDebugEventListener removedListener; + uint64_t removedListenerCalls {}; + removingListener.OnDebugEventOverride = [&](DebugEvent&) noexcept { + source.RemoveEventListener(EVT_LOG_EVENT, removedListener); + }; + removedListener.OnDebugEventOverride = [&](DebugEvent&) noexcept { + removedListenerCalls++; + }; + source.AddEventListener(EVT_LOG_EVENT, removingListener); + source.AddEventListener(EVT_LOG_EVENT, removedListener); + + source.DispatchEvent(DebugEvent { EVT_LOG_EVENT }); + source.DispatchEvent(DebugEvent { EVT_LOG_EVENT }); + + ASSERT_EQ(removedListenerCalls, uint64_t { 1 }); +} + TEST(DebugEventSourceTests, DispatchEvent_OneEventToCascaded_ListenerSeesOneEvent) { TestDebugEventSource source; @@ -217,4 +238,3 @@ TEST(DebugEventSourceTests, DispatchEvent_OneEventToCascadedAndToSource_Listener ASSERT_EQ(sequenceNumberToCountMap[1], uint64_t { 2 }); } - From 7d17e6cff31a6a5cae7261fee94350e62c14cb2f Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 28 Aug 2026 17:17:10 -0500 Subject: [PATCH 3/5] jni: register pending release callback lazily Avoid cross-translation-unit static initialization ordering by registering the callback once from nativeAddEventListener. Files changed: - lib/jni/LogManager_jni.cpp: replace eager registration with std::call_once. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7e12a191-38f1-48e1-baea-bdfc47d23030 --- lib/jni/LogManager_jni.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/jni/LogManager_jni.cpp b/lib/jni/LogManager_jni.cpp index d72ebb4b0..c6a0b6213 100644 --- a/lib/jni/LogManager_jni.cpp +++ b/lib/jni/LogManager_jni.cpp @@ -1787,11 +1787,14 @@ namespace ReleaseUnregisteredListener(callback); } - const bool pendingReleaseCallbackRegistered = [] { - SetDebugEventListenerPendingReleaseCallback( - ReleasePendingJniDebugEventListener); - return true; - }(); + void EnsurePendingReleaseCallbackRegistered() + { + static std::once_flag registerCallback; + std::call_once(registerCallback, [] { + SetDebugEventListenerPendingReleaseCallback( + ReleasePendingJniDebugEventListener); + }); + } } // anonymous namespace extern "C" JNIEXPORT jlong JNICALL @@ -1817,6 +1820,8 @@ Java_com_microsoft_applications_events_LogManagerProvider_00024LogManagerImpl_na try { + EnsurePendingReleaseCallbackRegistered(); + auto eventType = static_cast(event_type); std::shared_ptr callback; jlong identity = -1; From b5ee5c2783f6d4cde35c7bf368b33eee8a0dbf51 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 28 Aug 2026 17:36:57 -0500 Subject: [PATCH 4/5] jni: release array element on lookup failure Prevent a local-reference leak when GetObjectArrayElement leaves a Java exception pending. Files changed: - lib/jni/JniConvertors.cpp: delete the returned local reference before the exceptional return. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7e12a191-38f1-48e1-baea-bdfc47d23030 --- lib/jni/JniConvertors.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/jni/JniConvertors.cpp b/lib/jni/JniConvertors.cpp index 9f537f21e..c8931cae6 100644 --- a/lib/jni/JniConvertors.cpp +++ b/lib/jni/JniConvertors.cpp @@ -208,6 +208,7 @@ std::vector ConvertJObjectArrayToStdStringVector(JNIEnv* env, const auto jStringValue = static_cast(env->GetObjectArrayElement(jArrayToConvert, i)); if (env->ExceptionCheck()) { + env->DeleteLocalRef(jStringValue); return stringVector; } auto stringValue = JStringToStdString(env, jStringValue); From dd062fadd8f87b44ed486ac892fc013f1e0af1c1 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 28 Aug 2026 17:57:44 -0500 Subject: [PATCH 5/5] tests: bypass mock when exercising upload implementation Call uploadAsyncParent directly so implementation tests do not trigger an uninteresting gMock call through the forwarding default action. Files changed: - tests/unittests/TransmissionPolicyManagerTests.cpp: use the existing parent helper in upload implementation tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7e12a191-38f1-48e1-baea-bdfc47d23030 --- tests/unittests/TransmissionPolicyManagerTests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unittests/TransmissionPolicyManagerTests.cpp b/tests/unittests/TransmissionPolicyManagerTests.cpp index 6cbdb99f5..c2ce2c3ae 100644 --- a/tests/unittests/TransmissionPolicyManagerTests.cpp +++ b/tests/unittests/TransmissionPolicyManagerTests.cpp @@ -274,7 +274,7 @@ TEST_F(TransmissionPolicyManagerTests, UploadPostponedWithInsufficientAvailableB .WillOnce(Return(999999)); EXPECT_CALL(tpm, scheduleUpload(1000, EventLatency_Normal, false)) .WillOnce(Return()); - tpm.uploadAsync(EventLatency_Normal); + tpm.uploadAsyncParent(EventLatency_Normal); EXPECT_THAT(tpm.uploadScheduled(), false); } @@ -288,7 +288,7 @@ TEST_F(TransmissionPolicyManagerTests, UploadInitiatesUpload) EventsUploadContextPtr upload; EXPECT_CALL(*this, resultInitiateUpload(_)) .WillOnce(SaveArg<0>(&upload)); - tpm.uploadAsync(EventLatency_Normal); + tpm.uploadAsyncParent(EventLatency_Normal); EXPECT_THAT(tpm.uploadScheduled(), false); EXPECT_THAT(upload, NotNull());