Fix runtime data races, memory leak, and shutdown safety

lib/api/LogConfiguration.cpp

@@ -19,6 +19,7 @@ namespace MAT_NS_BEGIN {
         { CFG_BOOL_ENABLE_ANALYTICS,        false },
         { CFG_INT_CACHE_FILE_SIZE,          3145728 },
         { CFG_INT_RAM_QUEUE_SIZE,           524288 },
+        { CFG_BOOL_ENABLE_BATCHED_STORAGE_FLUSH, true },
         { CFG_BOOL_ENABLE_MULTITENANT,      true },
         { CFG_BOOL_ENABLE_DB_DROP_IF_FULL,  false },
         { CFG_INT_MAX_TEARDOWN_TIME,        0 },
@@ -51,6 +52,7 @@ namespace MAT_NS_BEGIN {
             { CFG_BOOL_ENABLE_ANALYTICS,    src.enableLifecycleSession },
             { CFG_INT_CACHE_FILE_SIZE,      src.cacheFileSizeLimitInBytes },
             { CFG_INT_RAM_QUEUE_SIZE,       src.cacheMemorySizeLimitInBytes },
+            { CFG_BOOL_ENABLE_BATCHED_STORAGE_FLUSH, true },
             { CFG_BOOL_ENABLE_MULTITENANT,  src.multiTenantEnabled },
             { CFG_INT_MAX_TEARDOWN_TIME,    src.maxTeardownUploadTimeInSec },
             { CFG_INT_MAX_PENDING_REQ,      src.maxPendingHTTPRequests },
@@ -128,4 +130,3 @@ namespace MAT_NS_BEGIN {
     }
 
 } MAT_NS_END
-

lib/api/LogManagerFactory.hpp

@@ -67,7 +67,15 @@ namespace MAT_NS_BEGIN {
 
         //  C++11 Magic Statics (N2660)
         static LogManagerFactory& instance() {
-            static LogManagerFactory impl;
+            // Deliberately never destroyed. LogManagerProvider::Release() must be
+            // able to walk this factory's registries during process teardown, but
+            // a normal function-local static's destruction order relative to that
+            // teardown call is unspecified -- if this were destroyed first,
+            // Release() would walk already-freed std::map nodes (a downstream
+            // consumer observed this as EXC_BAD_ACCESS in release() at process
+            // exit). Leaking one small, fixed-size object avoids the ordering
+            // hazard entirely; the OS reclaims it when the process exits.
+            static LogManagerFactory& impl = *new LogManagerFactory();
             return impl;
         }
 

lib/api/LogManagerImpl.cpp

@@ -7,6 +7,7 @@
 #pragma warning(disable : 4459)
 #endif
 #include "LogManagerImpl.hpp"
+#include <cstdio>
 #include "mat/config.h"
 
 #include "offline/LogSessionDataProvider.hpp"
@@ -368,9 +369,27 @@ namespace MAT_NS_BEGIN
 
     LogManagerImpl::~LogManagerImpl() noexcept
     {
-        FlushAndTeardown();
-        LOCKGUARD(ILogManagerInternal::managers_lock);
-        ILogManagerInternal::managers.erase(this);
+        try
+        {
+            FlushAndTeardown();
+        }
+        catch (const std::exception& e)
+        {
+            std::fprintf(stderr, "Log manager teardown failed: %s\n", e.what());
+        }
+        catch (...)
+        {
+            std::fputs("Log manager teardown failed with an unknown exception\n", stderr);
+        }
+        try
+        {
+            LOCKGUARD(ILogManagerInternal::managers_lock);
+            ILogManagerInternal::managers.erase(this);
+        }
+        catch (...)
+        {
+            std::fputs("Log manager registry cleanup failed\n", stderr);
+        }
     }
 
     size_t LogManagerImpl::GetDeadLoggerCount()
@@ -959,19 +978,30 @@ namespace MAT_NS_BEGIN
         return true;
     }
 
-    void LogManagerImpl::EndActivity()
+    void LogManagerImpl::EndActivity() noexcept
     {
-        std::unique_lock<std::mutex> lock(m_pause_mutex);
-        if (m_pause_active_count == 0) {
-            return;
+        try
+        {
+            std::unique_lock<std::mutex> lock(m_pause_mutex);
+            if (m_pause_active_count == 0) {
+                return;
+            }
+            m_pause_active_count -= 1;
+            if (m_pause_active_count > 0) {
+                return;
+            }
+            if (m_pause_state == PauseState::Pausing) {
+                m_pause_state = PauseState::Paused;
+                m_pause_cv.notify_all();
+            }
         }
-        m_pause_active_count -= 1;
-        if (m_pause_active_count > 0) {
-            return;
+        catch (const std::exception& e)
+        {
+            std::fprintf(stderr, "Failed to end telemetry activity: %s\n", e.what());
         }
-        if (m_pause_state == PauseState::Pausing) {
-            m_pause_state = PauseState::Paused;
-            m_pause_cv.notify_all();
+        catch (...)
+        {
+            std::fputs("Failed to end telemetry activity\n", stderr);
         }
     }
 }

lib/api/LogManagerImpl.hpp

@@ -306,7 +306,7 @@ namespace MAT_NS_BEGIN
         virtual void ResumeActivity() override;
         virtual void WaitPause() override;
         virtual bool StartActivity() override;
-        virtual void EndActivity() override;
+        virtual void EndActivity() noexcept override;
 
        protected:
         std::unique_ptr<ITelemetrySystem>& GetSystem();

lib/api/Logger.cpp

@@ -127,7 +127,8 @@ namespace MAT_NS_BEGIN
 
     Logger::~Logger() noexcept
     {
-        LOG_TRACE("%p: Destroyed", this);
+        // Intentionally empty — logging here triggers a static-destruction-order
+        // crash on iOS simulator (recursive_mutex used after teardown).
     }
 
     ISemanticContext* Logger::GetSemanticContext() const

lib/config/RuntimeConfig_Default.hpp

@@ -16,6 +16,7 @@ namespace MAT_NS_BEGIN
         {CFG_BOOL_ENABLE_ANALYTICS, false},
         {CFG_INT_CACHE_FILE_SIZE, 3145728},
         {CFG_INT_RAM_QUEUE_SIZE, 524288},
+        {CFG_BOOL_ENABLE_BATCHED_STORAGE_FLUSH, true},
         {CFG_BOOL_ENABLE_MULTITENANT, true},
         {CFG_BOOL_ENABLE_DB_DROP_IF_FULL, false},
         {CFG_INT_MAX_TEARDOWN_TIME, 1},
@@ -233,4 +234,3 @@ namespace MAT_NS_BEGIN
 
 }
 MAT_NS_END
-

lib/http/HttpClient_Apple.mm

@@ -207,9 +207,11 @@ void HandleResponse(NSData* data, NSURLResponse* response, NSError* error)
             NSHTTPURLResponse *httpResp = static_cast<NSHTTPURLResponse*>(response);
             auto simpleResponse = new SimpleHttpResponse { NextRespId() };
 
-            simpleResponse->m_statusCode = static_cast<unsigned int>(httpResp.statusCode);
+            simpleResponse->m_statusCode = httpResp != nil
+                ? static_cast<unsigned int>(httpResp.statusCode)
+                : 0;
 
-            NSDictionary *responseHeaders = [httpResp allHeaderFields];
+            NSDictionary *responseHeaders = httpResp != nil ? [httpResp allHeaderFields] : nil;
             for (id key in responseHeaders)
             {
                 simpleResponse->m_headers.add([key UTF8String], [responseHeaders[key] UTF8String]);
@@ -299,7 +301,6 @@ void Cancel()
                 LOG_TRACE("HTTP request=%p id=%s being aborted...", request, id.c_str());
                 request->Cancel();
             }
-            m_requests.erase(id);
         }
     }
 }
@@ -317,17 +318,6 @@ void Cancel()
     for (const auto &id : ids)
         CancelRequestAsync(id);
 
-    for (;;)
-    {
-        {
-            std::lock_guard<std::mutex> lock(m_requestsMtx);
-            if (m_requests.empty())
-            {
-                return;
-            }
-        }
-        PAL::sleep(100);
-    }
 }
 
 void HttpClient_Apple::Erase(IHttpRequest* req)

lib/http/HttpClient_WinInet.cpp

@@ -55,6 +55,9 @@ class WinInetRequestWrapper
         if (m_hWinInetRequest != nullptr)
         {
             ::InternetCloseHandle(m_hWinInetRequest);
+        }
+        if (m_hWinInetSession != nullptr)
+        {
             ::InternetCloseHandle(m_hWinInetSession);
         }
     }

lib/http/HttpResponseDecoder.cpp

@@ -67,13 +67,11 @@ namespace MAT_NS_BEGIN {
             break;
 
         case HttpResult_Aborted:
-            ctx->httpResponse = nullptr;
             outcome = Abort;
             break;
 
         case HttpResult_LocalFailure:
         case HttpResult_NetworkFailure:
-            ctx->httpResponse = nullptr;
             outcome = RetryNetwork;
             break;
         }
@@ -132,7 +130,6 @@ namespace MAT_NS_BEGIN {
                 evt.param2 = ctx->recordIdsAndTenantIds.size();
                 DispatchEvent(evt);
             }
-            ctx->httpResponse = nullptr;
             // eventsRejected(ctx); // FIXME: [MG] - investigate why ctx gets corrupt after eventsRejected
             requestAborted(ctx);
             break;

lib/include/public/ILogConfiguration.hpp

@@ -154,6 +154,12 @@ namespace MAT_NS_BEGIN
     /// </summary>
     static constexpr const char* const CFG_INT_RAM_QUEUE_BUFFERS = "maxDBFlushQueues";
 
+    /// <summary>
+    /// Batch records when flushing the RAM queue to disk storage.
+    /// Set to false to use per-record disk stores during flush.
+    /// </summary>
+    static constexpr const char* const CFG_BOOL_ENABLE_BATCHED_STORAGE_FLUSH = "enableBatchedStorageFlush";
+
     /// <summary>
     /// SQLite DB will be checkpointed when flushing.
     /// </summary>
@@ -481,4 +487,3 @@ namespace MAT_NS_BEGIN
 }
 MAT_NS_END
 #endif
-

lib/include/public/ITaskDispatcher.hpp

@@ -121,11 +121,11 @@ namespace MAT_NS_BEGIN
         /// <param name="waitTime">Amount of time to wait for if the task is currently executing</param>
         /// <returns>True if successfully cancelled, else false</returns>
         virtual bool Cancel(Task* task, uint64_t waitTime = 0) = 0;
+
     };
 
     /// @endcond
 
 } MAT_NS_END
 
 #endif // ITASKDISPATCHER_HPP
-

lib/jni/PrivacyGuard_jni.cpp

@@ -62,16 +62,22 @@ Java_com_microsoft_applications_events_PrivacyGuard_nativeInitializePrivacyGuard
     InitializationConfiguration config(
             reinterpret_cast<ILogger*>(iLoggerNativePtr),
             CommonDataContext{});
+    // InitializationConfiguration holds const char* pointers, so the backing
+    // std::string storage must outlive the PrivacyGuard construction below.
+    std::string notificationEventName, semanticContextEventName, summaryEventName;
     if (NotificationEventName != nullptr) {
-        config.NotificationEventName = JStringToStdString(env, NotificationEventName).c_str();
+        notificationEventName = JStringToStdString(env, NotificationEventName);
+        config.NotificationEventName = notificationEventName.c_str();
     }
 
     if (SemanticContextEventName != nullptr) {
-        config.SemanticContextNotificationEventName = JStringToStdString(env, SemanticContextEventName).c_str();
+        semanticContextEventName = JStringToStdString(env, SemanticContextEventName);
+        config.SemanticContextNotificationEventName = semanticContextEventName.c_str();
     }
 
     if (SummaryEventName != nullptr) {
-        config.SummaryEventName = JStringToStdString(env, SummaryEventName).c_str();
+        summaryEventName = JStringToStdString(env, SummaryEventName);
+        config.SummaryEventName = summaryEventName.c_str();
     }
 
     config.UseEventFieldPrefix = static_cast<bool>(UseEventFieldPrefix);
@@ -119,16 +125,22 @@ Java_com_microsoft_applications_events_PrivacyGuard_nativeInitializePrivacyGuard
                                             machineIds,
                                             outOfScopeIdentifiers));
 
+    // InitializationConfiguration holds const char* pointers, so the backing
+    // std::string storage must outlive the PrivacyGuard construction below.
+    std::string notificationEventName, semanticContextEventName, summaryEventName;
     if (NotificationEventName != NULL) {
-        config.NotificationEventName = JStringToStdString(env, NotificationEventName).c_str();
+        notificationEventName = JStringToStdString(env, NotificationEventName);
+        config.NotificationEventName = notificationEventName.c_str();
     }
 
     if (SemanticContextEventName != NULL) {
-        config.SemanticContextNotificationEventName = JStringToStdString(env, SemanticContextEventName).c_str();
+        semanticContextEventName = JStringToStdString(env, SemanticContextEventName);
+        config.SemanticContextNotificationEventName = semanticContextEventName.c_str();
     }
 
     if (SummaryEventName != NULL) {
-        config.SummaryEventName = JStringToStdString(env, SummaryEventName).c_str();
+        summaryEventName = JStringToStdString(env, SummaryEventName);
+        config.SummaryEventName = summaryEventName.c_str();
     }
 
     config.UseEventFieldPrefix = static_cast<bool>(UseEventFieldPrefix);

lib/offline/MemoryStorage.cpp

@@ -224,6 +224,16 @@ namespace MAT_NS_BEGIN {
 
     void MemoryStorage::DeleteRecords(const std::map<std::string, std::string> & whereFilter)
     {
+        // An empty filter matches every record. Never silently wipe the whole
+        // in-memory queue from a no-op predicate; callers must use
+        // DeleteAllRecords() for an intentional full clear. This mirrors the
+        // fail-closed behavior of OfflineStorage_SQLite::DeleteRecords.
+        if (whereFilter.empty())
+        {
+            LOG_WARN("DeleteRecords called with an empty filter; ignoring to avoid deleting all records.");
+            return;
+        }
+
         auto matcher = [&](const StorageRecord &r, const std::map<std::string, std::string> & whereFilter)
         {
             bool matched = true;