From 66faca8d506351f0559e65e475897109d631afce Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Sun, 8 Nov 2026 16:17:44 -0600 Subject: [PATCH 1/3] Keep late telemetry workers safe in embedded SDK copies Retain the SDK version and auth-token lock through exit-owner teardown. Honor Apple system SQLite/zlib even with vcpkg, and let independent embedded copies select a private Objective-C HTTP delegate name without changing the default. Files: CMakeLists.txt; cmake/MatsdkOptions.cmake; docs/embedding-with-cmake.md; lib/CMakeLists.txt; lib/api/AuthTokensController.cpp; lib/pal/PAL.cpp; tests/apple/embedding-options-tests.cmake; tests/apple/embedding-options/CMakeLists.txt; tests/embedding/CMakeLists.txt; tests/embedding/lifetime-tests.cmake; tests/embedding/lifetime-tests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5484aed9-9464-4bc8-9ca7-fd67954034ff --- CMakeLists.txt | 4 +- cmake/MatsdkOptions.cmake | 5 + docs/embedding-with-cmake.md | 16 +++- lib/CMakeLists.txt | 5 + lib/api/AuthTokensController.cpp | 8 +- lib/pal/PAL.cpp | 8 +- tests/apple/embedding-options-tests.cmake | 43 +++++++++ tests/apple/embedding-options/CMakeLists.txt | 70 ++++++++++++++ tests/embedding/CMakeLists.txt | 7 ++ tests/embedding/lifetime-tests.cmake | 52 +++++++++++ tests/embedding/lifetime-tests.cpp | 98 ++++++++++++++++++++ 11 files changed, 309 insertions(+), 7 deletions(-) create mode 100644 tests/apple/embedding-options-tests.cmake create mode 100644 tests/apple/embedding-options/CMakeLists.txt create mode 100644 tests/embedding/lifetime-tests.cmake create mode 100644 tests/embedding/lifetime-tests.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index f56eb8f7f..105ceb21b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -228,7 +228,7 @@ if(TARGET SQLite::SQLite3 AND NOT TARGET SQLite3::SQLite3) matsdk_add_interface_dependency(SQLite3::SQLite3 SQLite::SQLite3) endif() if(MATSDK_SQLITE_PROVIDER_RESOLVED STREQUAL "SYSTEM" AND NOT TARGET SQLite3::SQLite3) - if(APPLE AND NOT MATSDK_USING_VCPKG) + if(APPLE) matsdk_add_apple_system_library(SQLite3::SQLite3 sqlite3) set(MATSDK_SQLITE_DEPENDENCY_KIND APPLE_SYSTEM) else() @@ -253,7 +253,7 @@ if(MATSDK_SQLITE_PROVIDER_RESOLVED STREQUAL "SYSTEM" AND NOT TARGET SQLite3::SQL endif() if(MATSDK_ZLIB_PROVIDER_RESOLVED STREQUAL "SYSTEM" AND NOT TARGET ZLIB::ZLIB) - if(APPLE AND NOT MATSDK_USING_VCPKG) + if(APPLE) matsdk_add_apple_system_library(ZLIB::ZLIB z) set(MATSDK_ZLIB_DEPENDENCY_KIND APPLE_SYSTEM) else() diff --git a/cmake/MatsdkOptions.cmake b/cmake/MatsdkOptions.cmake index ac8a38c7e..41fd4812b 100644 --- a/cmake/MatsdkOptions.cmake +++ b/cmake/MatsdkOptions.cmake @@ -51,6 +51,11 @@ option(MATSDK_BUILD_AZMON "Build Azure Monitor / Application Insights support" ON) option(MATSDK_BUILD_APPLE_HTTP "Build the Apple-native HTTP client" "${APPLE}") +set(MATSDK_APPLE_HTTP_DELEGATE_CLASS "MATStreamingSessionDelegate" CACHE STRING + "Private Objective-C class name for the Apple HTTP session delegate") +if(NOT MATSDK_APPLE_HTTP_DELEGATE_CLASS MATCHES "^[A-Za-z_][A-Za-z0-9_]*$") + message(FATAL_ERROR "MATSDK_APPLE_HTTP_DELEGATE_CLASS must be a C identifier.") +endif() option(MATSDK_USE_WININET "Use WinInet instead of WinHTTP as the Win32 desktop HTTP client" OFF) option(MATSDK_DISABLE_LOGGING diff --git a/docs/embedding-with-cmake.md b/docs/embedding-with-cmake.md index 692d91579..7bcd58f0a 100644 --- a/docs/embedding-with-cmake.md +++ b/docs/embedding-with-cmake.md @@ -61,7 +61,8 @@ Recommended packaged-library policy: | Android | `MINIMAL`, or `NONE` with Room | `VENDORED` | Java/JNI by default | Apple's SQLite and zlib entries are system libraries: consumers link them but -do not ship private copies. A Linux host such as Foundry Local that already +do not ship private copies, including when a vcpkg toolchain is active. Existing +caller-provided canonical targets still take precedence. A Linux host such as Foundry Local that already standardizes on libcurl/OpenSSL should provide `CURL::libcurl` and select `MATSDK_CURL_PROVIDER=SYSTEM`; other self-contained Linux consumers can use the SDK's pinned curl/mbedTLS build. @@ -71,6 +72,19 @@ consumer must set `skipSqliteInitAndShutdown` to `"true"` and leave SQLite's process-wide lifetime to the host. This is the required configuration for coexisting Apple libraries that all link the system `libsqlite3`. +When independently embedded SDK copies coexist in an Apple process, choose a +different private HTTP delegate class name for each copy before adding the SDK: + +```cmake +set(MATSDK_APPLE_HTTP_DELEGATE_CLASS MyProductMATStreamingSessionDelegate + CACHE STRING "" FORCE) +``` + +The default remains `MATStreamingSessionDelegate`. Objective-C class names are +process-global even when C++ symbols are hidden. This setting affects only the +Apple HTTP implementation and does not propagate to consumers or rename public +Objective-C wrapper classes. + ## Non-vcpkg dependency selection When the CPP11 PAL uses the curl HTTP transport outside vcpkg, the SDK normally diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index d44de8ab6..43eb05d07 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -376,6 +376,11 @@ else() message(STATUS "Building static SDK library") add_library(mat STATIC ${SRCS}) endif() +if(APPLE AND MATSDK_BUILD_APPLE_HTTP) + # Objective-C class names are process-global, even with hidden symbol visibility. + target_compile_definitions(mat PRIVATE + "MATStreamingSessionDelegate=${MATSDK_APPLE_HTTP_DELEGATE_CLASS}") +endif() target_link_libraries(mat PRIVATE $ $) diff --git a/lib/api/AuthTokensController.cpp b/lib/api/AuthTokensController.cpp index 4a7778aaa..93c0ba806 100644 --- a/lib/api/AuthTokensController.cpp +++ b/lib/api/AuthTokensController.cpp @@ -6,6 +6,7 @@ #include "AuthTokensController.hpp" #include "ILogManager.hpp" #include "utils/Utils.hpp" +#include namespace MAT_NS_BEGIN { @@ -24,8 +25,11 @@ namespace MAT_NS_BEGIN { std::mutex& AuthTokensController::GetMutex() { - static std::mutex mutex; - return mutex; + // Like GetPAL(), this lock must survive late upload workers. + using AuthMutex = std::mutex; + alignas(AuthMutex) static unsigned char storage[sizeof(AuthMutex)]; + static AuthMutex* mutex = ::new (storage) AuthMutex; + return *mutex; } status_t AuthTokensController::SetTicketToken(TicketType type, char const* tokenValue) diff --git a/lib/pal/PAL.cpp b/lib/pal/PAL.cpp index 94a71b4a0..abd7ebe9d 100644 --- a/lib/pal/PAL.cpp +++ b/lib/pal/PAL.cpp @@ -631,8 +631,12 @@ namespace PAL_NS_BEGIN { const std::string& PlatformAbstractionLayer::getSdkVersion() const { - static const std::string sdkVersion(EVTSDK_VERSION_PREFIX "-" OS_NAME "-C++-" ECS_SUPP "-" BUILD_VERSION_STR); - return sdkVersion; + // Like GetPAL(), this immutable value must survive late upload workers. + using SdkVersion = std::string; + alignas(SdkVersion) static unsigned char storage[sizeof(SdkVersion)]; + static const SdkVersion* sdkVersion = ::new (storage) + SdkVersion(EVTSDK_VERSION_PREFIX "-" OS_NAME "-C++-" ECS_SUPP "-" BUILD_VERSION_STR); + return *sdkVersion; } void PlatformAbstractionLayer::initialize(IRuntimeConfig& configuration) diff --git a/tests/apple/embedding-options-tests.cmake b/tests/apple/embedding-options-tests.cmake new file mode 100644 index 000000000..01a203e49 --- /dev/null +++ b/tests/apple/embedding-options-tests.cmake @@ -0,0 +1,43 @@ +cmake_minimum_required(VERSION 3.19) +get_filename_component(sdk_root "${CMAKE_CURRENT_LIST_DIR}/../.." ABSOLUTE) +if(NOT DEFINED OUTPUT_DIR) + set(OUTPUT_DIR "${sdk_root}/out/apple-embedding-options") +endif() +set(generator_args) +if(TEST_GENERATOR) + list(APPEND generator_args -G "${TEST_GENERATOR}") +endif() +if(TEST_GENERATOR_PLATFORM) + list(APPEND generator_args -A "${TEST_GENERATOR_PLATFORM}") +endif() + +function(check_options apple vcpkg provided http class) + # Keep MSBuild's nested compiler-probe paths below its legacy path limit. + string(SHA1 name "${apple}-${vcpkg}-${provided}-${http}-${class}") + string(SUBSTRING "${name}" 0 6 name) + execute_process(COMMAND "${CMAKE_COMMAND}" ${generator_args} + -S "${CMAKE_CURRENT_LIST_DIR}/embedding-options" -B "${OUTPUT_DIR}/${name}" + "-DSDK_ROOT=${sdk_root}" "-DTEST_APPLE=${apple}" "-DTEST_VCPKG=${vcpkg}" + "-DTEST_PROVIDED=${provided}" "-DTEST_HTTP=${http}" + "-DMATSDK_APPLE_HTTP_DELEGATE_CLASS=${class}" + RESULT_VARIABLE result OUTPUT_VARIABLE output ERROR_VARIABLE error) + if(class STREQUAL "invalid-class") + if(result EQUAL 0 OR NOT error MATCHES "must be a C identifier") + message(FATAL_ERROR "Invalid delegate name was not rejected:\n${output}\n${error}") + endif() + elseif(NOT result EQUAL 0) + message(FATAL_ERROR "${apple}/${vcpkg}/${provided}/${http}/${class} failed:\n${output}\n${error}") + endif() +endfunction() + +foreach(apple ON OFF) + foreach(vcpkg ON OFF) + foreach(provided ON OFF) + check_options("${apple}" "${vcpkg}" "${provided}" ON MATStreamingSessionDelegate) + check_options("${apple}" "${vcpkg}" "${provided}" ON ProductMATStreamingSessionDelegate) + endforeach() + endforeach() +endforeach() +check_options(ON ON OFF OFF ProductMATStreamingSessionDelegate) +check_options(ON ON OFF ON invalid-class) +message(STATUS "18 Apple dependency/delegate configurations passed") diff --git a/tests/apple/embedding-options/CMakeLists.txt b/tests/apple/embedding-options/CMakeLists.txt new file mode 100644 index 000000000..0188837f4 --- /dev/null +++ b/tests/apple/embedding-options/CMakeLists.txt @@ -0,0 +1,70 @@ +cmake_minimum_required(VERSION 3.19) +project(apple_embedding_options LANGUAGES C CXX) + +file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/platform.cmake" "set(APPLE ${TEST_APPLE})\n") +set(CMAKE_PROJECT_INCLUDE "${CMAKE_CURRENT_BINARY_DIR}/platform.cmake") +set(APPLE "${TEST_APPLE}") +if(TEST_VCPKG) + set(VCPKG_TARGET_TRIPLET test-triplet) +endif() +set(MATSDK_BUILD_HEADERS OFF CACHE BOOL "" FORCE) +set(MATSDK_BUILD_LIBRARY OFF CACHE BOOL "" FORCE) +set(MATSDK_BUILD_TEST_TOOL OFF CACHE BOOL "" FORCE) +set(MATSDK_BUILD_UNIT_TESTS OFF CACHE BOOL "" FORCE) +set(MATSDK_BUILD_FUNC_TESTS OFF CACHE BOOL "" FORCE) +set(MATSDK_BUILD_PACKAGE OFF CACHE BOOL "" FORCE) +set(MATSDK_BUILD_APPLE_HTTP "${TEST_HTTP}" CACHE BOOL "" FORCE) +set(MATSDK_SQLITE_PROVIDER SYSTEM CACHE STRING "" FORCE) +set(MATSDK_ZLIB_PROVIDER SYSTEM CACHE STRING "" FORCE) +add_library(CURL::libcurl INTERFACE IMPORTED) +add_library(nlohmann_json::nlohmann_json INTERFACE IMPORTED) +if(TEST_PROVIDED OR NOT TEST_APPLE) + add_library(SQLite3::SQLite3 INTERFACE IMPORTED) + add_library(ZLIB::ZLIB INTERFACE IMPORTED) + set_property(TARGET SQLite3::SQLite3 PROPERTY INTERFACE_LINK_LIBRARIES caller-sqlite) + set_property(TARGET ZLIB::ZLIB PROPERTY INTERFACE_LINK_LIBRARIES caller-zlib) +endif() +foreach(package SQLite3 unofficial-sqlite3 ZLIB nlohmann_json) + set(CMAKE_DISABLE_FIND_PACKAGE_${package} ON) +endforeach() +add_subdirectory("${SDK_ROOT}" sdk) +foreach(pair "SQLite3::SQLite3|sqlite3|caller-sqlite" "ZLIB::ZLIB|z|caller-zlib") + string(REPLACE "|" ";" pair "${pair}") + list(GET pair 0 target) + list(GET pair 1 expected) + if(TEST_PROVIDED OR NOT TEST_APPLE) + list(GET pair 2 expected) + endif() + get_target_property(links "${target}" INTERFACE_LINK_LIBRARIES) + if(NOT links STREQUAL expected) + message(FATAL_ERROR "${target} links ${links}, expected ${expected}") + endif() +endforeach() + +# Evaluate the production target guard without requiring an Apple compiler or SDK. +file(READ "${SDK_ROOT}/lib/CMakeLists.txt" library) +string(FIND "${library}" "if(APPLE AND MATSDK_BUILD_APPLE_HTTP)" start) +if(start EQUAL -1) + message(FATAL_ERROR "Update Apple delegate target instrumentation") +endif() +string(SUBSTRING "${library}" ${start} -1 definition) +string(FIND "${definition}" "\nendif()" end) +math(EXPR end "${end} + 8") +string(SUBSTRING "${definition}" 0 ${end} definition) +file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/delegate.cpp" "") +add_library(mat STATIC "${CMAKE_CURRENT_BINARY_DIR}/delegate.cpp") +file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/delegate.cmake" "${definition}") +include("${CMAKE_CURRENT_BINARY_DIR}/delegate.cmake") +get_target_property(private mat COMPILE_DEFINITIONS) +get_target_property(public mat INTERFACE_COMPILE_DEFINITIONS) +set(expected "MATStreamingSessionDelegate=${MATSDK_APPLE_HTTP_DELEGATE_CLASS}") +if(TEST_APPLE AND TEST_HTTP) + if(NOT expected IN_LIST private) + message(FATAL_ERROR "Missing private Apple delegate definition: ${private}") + endif() +elseif(expected IN_LIST private) + message(FATAL_ERROR "Delegate definition leaked into a non-Apple HTTP build") +endif() +if(expected IN_LIST public) + message(FATAL_ERROR "Delegate definition propagated to consumers") +endif() diff --git a/tests/embedding/CMakeLists.txt b/tests/embedding/CMakeLists.txt index fb7a01ffa..9b1a4cc8f 100644 --- a/tests/embedding/CMakeLists.txt +++ b/tests/embedding/CMakeLists.txt @@ -114,3 +114,10 @@ endif() enable_testing() add_test(NAME embedding_test COMMAND embedding_test) +include("${CMAKE_CURRENT_LIST_DIR}/lifetime-tests.cmake") +add_test(NAME apple_embedding_options + COMMAND "${CMAKE_COMMAND}" + "-DOUTPUT_DIR=${CMAKE_CURRENT_BINARY_DIR}/apple-options" + "-DTEST_GENERATOR=${CMAKE_GENERATOR}" + "-DTEST_GENERATOR_PLATFORM=${CMAKE_GENERATOR_PLATFORM}" + -P "${CMAKE_CURRENT_LIST_DIR}/../apple/embedding-options-tests.cmake") diff --git a/tests/embedding/lifetime-tests.cmake b/tests/embedding/lifetime-tests.cmake new file mode 100644 index 000000000..9db97aac4 --- /dev/null +++ b/tests/embedding/lifetime-tests.cmake @@ -0,0 +1,52 @@ +# Static embedding lets the instrumented production objects replace their archive counterparts. +if(BUILD_SHARED_LIBS) + return() +endif() + +set(lifetime_dir "${CMAKE_CURRENT_BINARY_DIR}/lifetime-sdk") +file(MAKE_DIRECTORY "${lifetime_dir}") +foreach(source pal/PAL.cpp api/AuthTokensController.cpp) + set(source_path "${sdk_source}/lib/${source}") + set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS "${source_path}") + file(READ "${source_path}" instrumented) + if(source STREQUAL "pal/PAL.cpp") + string(REPLACE "static const std::string sdkVersion(" "static const LifetimeString sdkVersion(" + instrumented "${instrumented}") + string(REPLACE "using SdkVersion = std::string;" "using SdkVersion = LifetimeString;" + instrumented "${instrumented}") + set(hook "LifetimeVersionDestroyed") + set(type String) + set(base string) + else() + string(REPLACE "static std::mutex mutex;" "static LifetimeMutex mutex;" + instrumented "${instrumented}") + string(REPLACE "using AuthMutex = std::mutex;" "using AuthMutex = LifetimeMutex;" + instrumented "${instrumented}") + set(hook "LifetimeMutexDestroyed") + set(type Mutex) + set(base mutex) + endif() + if(NOT instrumented MATCHES "(static const LifetimeString sdkVersion|using SdkVersion = LifetimeString|static LifetimeMutex mutex|using AuthMutex = LifetimeMutex)") + message(FATAL_ERROR "Update lifetime instrumentation for ${source}") + endif() + string(PREPEND instrumented + "#include \n#include \n" + "extern \"C\" void ${hook}();\n" + "struct Lifetime${type} : std::${base} {\n" + " using std::${base}::${base};\n" + " ~Lifetime${type}() { ${hook}(); }\n" + "};\n") + get_filename_component(name "${source}" NAME) + file(WRITE "${lifetime_dir}/${name}" "${instrumented}") +endforeach() + +add_executable(sdk_lifetime_tests "${CMAKE_CURRENT_LIST_DIR}/lifetime-tests.cpp" + "${lifetime_dir}/PAL.cpp" "${lifetime_dir}/AuthTokensController.cpp") +target_link_libraries(sdk_lifetime_tests PRIVATE MSTelemetry::mat matsdk_internal_config matsdk_build_options) +target_include_directories(sdk_lifetime_tests PRIVATE "$" + "${sdk_source}/lib/api") +add_dependencies(embedding_test sdk_lifetime_tests) +foreach(mode version mutex both) + add_test(NAME sdk_lifetime_${mode} COMMAND sdk_lifetime_tests "${mode}") + set_tests_properties(sdk_lifetime_${mode} PROPERTIES TIMEOUT 30) +endforeach() diff --git a/tests/embedding/lifetime-tests.cpp b/tests/embedding/lifetime-tests.cpp new file mode 100644 index 000000000..90df11436 --- /dev/null +++ b/tests/embedding/lifetime-tests.cpp @@ -0,0 +1,98 @@ +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +#include "api/AuthTokensController.hpp" +#include "pal/PAL.hpp" + +#include +#include +#include +#include +#include +#include + +namespace +{ + std::atomic versionDestroyed{false}; + std::atomic mutexDestroyed{false}; + bool checkVersion = false; + bool checkMutex = false; + const std::string* versionAddress = nullptr; + std::mutex* mutexAddress = nullptr; + + void Fail(const char* message) + { + std::fprintf(stderr, "%s\n", message); +#ifdef _WIN32 + // The exit owner holds the CRT exit lock while waiting for this worker. + ::TerminateProcess(::GetCurrentProcess(), 1); +#endif + std::_Exit(1); + } + + void ReadState() + { + if (checkVersion) + { + if (versionDestroyed.load()) + { + Fail("SDK version was destroyed before its exit owner"); + } + const auto& version = PAL::GetPAL().getSdkVersion(); + if (version.empty() || (versionAddress && versionAddress != &version)) + { + Fail("SDK version did not retain its value and identity"); + } + versionAddress = &version; + } + if (checkMutex) + { + if (mutexDestroyed.load()) + { + Fail("Auth mutex was destroyed before its exit owner"); + } + auto& mutex = MAT::AuthTokensController::GetMutex(); + if (mutexAddress && mutexAddress != &mutex) + { + Fail("Auth mutex did not retain its identity"); + } + std::lock_guard lock(mutex); + mutexAddress = &mutex; + } + } + + void ExitOwner() + { + // Model an owner registered before its first upload initializes SDK state. + std::thread lateWorker(ReadState); + lateWorker.join(); + } +} + +extern "C" void LifetimeVersionDestroyed() +{ + versionDestroyed.store(true); +} + +extern "C" void LifetimeMutexDestroyed() +{ + mutexDestroyed.store(true); +} + +int main(int argc, char** argv) +{ + if (argc != 2) + { + return 2; + } + checkVersion = std::strcmp(argv[1], "version") == 0 || std::strcmp(argv[1], "both") == 0; + checkMutex = std::strcmp(argv[1], "mutex") == 0 || std::strcmp(argv[1], "both") == 0; + if ((!checkVersion && !checkMutex) || std::atexit(ExitOwner) != 0) + { + return 2; + } + ReadState(); + ReadState(); + return 0; +} From 12461b9f742ff881f86f7b160db038ac60c1b520 Mon Sep 17 00:00:00 2001 From: bmehta001 Date: Sun, 8 Nov 2026 18:12:53 -0600 Subject: [PATCH 2/3] Bump version --- Package.swift | 4 ++-- lib/include/public/Version.hpp | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Package.swift b/Package.swift index cb993f9e1..1475eda87 100644 --- a/Package.swift +++ b/Package.swift @@ -104,8 +104,8 @@ let package = Package( name: "MATTelemetry", path: "build/apple/MATTelemetry.xcframework") : .binaryTarget( name: "MATTelemetry", - url: "https://github.com/microsoft/cpp_client_telemetry/releases/download/v3.10.309.1/MATTelemetry.xcframework.zip", - checksum: "12b20684bcf09f8daec9c5ee1a1ecd567c354d4dea3273ba7023a40b7d0c4589"), + url: "https://github.com/microsoft/cpp_client_telemetry/releases/download/v3.10.310.1/MATTelemetry.xcframework.zip", + checksum: "3d6d3b98e1696efce9bdad4d6eca177006b75b0a28fa866e75db6bf18e2cb8a9"), // Thin Swift API layer (source). Depends on the Obj-C module from the // xcframework. The conditional source exclusions above must stay in sync diff --git a/lib/include/public/Version.hpp b/lib/include/public/Version.hpp index aa94eb9ca..249077461 100644 --- a/lib/include/public/Version.hpp +++ b/lib/include/public/Version.hpp @@ -6,8 +6,8 @@ #define MAT_VERSION_HPP // WARNING: DO NOT MODIFY THIS FILE! // This file has been automatically generated, manual changes will be lost. -#define BUILD_VERSION_STR "3.10.310.1" -#define BUILD_VERSION 3,10,310,1 +#define BUILD_VERSION_STR "3.10.312.1" +#define BUILD_VERSION 3,10,312,1 #ifndef RESOURCE_COMPILER_INVOKED #include "ctmacros.hpp" @@ -18,7 +18,7 @@ namespace MAT_NS_BEGIN { uint64_t const Version = ((uint64_t)3 << 48) | ((uint64_t)10 << 32) | - ((uint64_t)310 << 16) | + ((uint64_t)312 << 16) | ((uint64_t)1); } MAT_NS_END From e630e2c9d11176499856393ffac9179520c80da6 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 9 Nov 2026 00:40:52 -0600 Subject: [PATCH 3/3] Avoid SDK version buffer leaks when embedded libraries unload The placement-new string avoided exit-time destruction but abandoned its heap buffer on unload: two real reload cycles leak 56 bytes under LSan. Return the compile-time literal through the internal const-char accessor instead, preserving late-worker safety without allocation or destruction. Keep the auth mutex and existing Apple embedding fixes unchanged. Files: - lib/pal/PAL.cpp, lib/pal/PAL.hpp: allocation-free internal version accessor. - tests/embedding/lifetime-tests.cpp: string-view value/address expectations. - tests/embedding/lifetime-tests.cmake: entry instrumentation and real DSO test. - tests/embedding/version-reload-fixture.cpp: embed the production SDK archive. - tests/embedding/version-reload-test.cpp: verify two actual unload cycles. - tests/embedding/CMakeLists.txt: opt-in Linux ASan/LSan coverage. - .github/workflows/test-embedding.yml: enforce sanitizer coverage in CI. Validation: Windows VS2026 and Linux GCC13 ASan/LSan embedding suites each pass all six tests. The old placement-new buffer leaks exactly 56 bytes; original destructible statics fail all three lifetime tests on both OSes. Formatting, existing misspell workflow and git diff --check pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5484aed9-9464-4bc8-9ca7-fd67954034ff --- .github/workflows/test-embedding.yml | 18 ++++ lib/pal/PAL.cpp | 10 +-- lib/pal/PAL.hpp | 4 +- tests/embedding/CMakeLists.txt | 19 +++++ tests/embedding/lifetime-tests.cmake | 35 +++++++- tests/embedding/lifetime-tests.cpp | 30 ++++++- tests/embedding/version-reload-fixture.cpp | 18 ++++ tests/embedding/version-reload-test.cpp | 96 ++++++++++++++++++++++ 8 files changed, 216 insertions(+), 14 deletions(-) create mode 100644 tests/embedding/version-reload-fixture.cpp create mode 100644 tests/embedding/version-reload-test.cpp diff --git a/.github/workflows/test-embedding.yml b/.github/workflows/test-embedding.yml index 2a0795866..9df6f47a5 100644 --- a/.github/workflows/test-embedding.yml +++ b/.github/workflows/test-embedding.yml @@ -94,6 +94,24 @@ jobs: cmake --build build-provided-consumer --parallel 4 ctest --test-dir build-provided-consumer --output-on-failure + linux-lifetime-sanitizers: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + - name: Install system dependencies + run: sudo apt-get update && sudo apt-get install -y libcurl4-openssl-dev libsqlite3-dev ninja-build zlib1g-dev + - name: Configure ASan and LSan embedding regressions + run: > + cmake -G Ninja -S tests/embedding -B build-lifetimes + -DCMAKE_BUILD_TYPE=RelWithDebInfo + -DMATSDK_EMBEDDING_ADDRESS_SANITIZER=ON + -DMATSDK_SQLITE_PROVIDER=SYSTEM + -DMATSDK_ZLIB_PROVIDER=SYSTEM + - name: Build and run + run: | + cmake --build build-lifetimes --target embedding_test --parallel 4 + ctest --test-dir build-lifetimes --output-on-failure + windows: runs-on: windows-2022 steps: diff --git a/lib/pal/PAL.cpp b/lib/pal/PAL.cpp index abd7ebe9d..3c3351f09 100644 --- a/lib/pal/PAL.cpp +++ b/lib/pal/PAL.cpp @@ -629,14 +629,10 @@ namespace PAL_NS_BEGIN { #define OS_NAME "Unknown" #endif - const std::string& PlatformAbstractionLayer::getSdkVersion() const + const char* PlatformAbstractionLayer::getSdkVersion() const { - // Like GetPAL(), this immutable value must survive late upload workers. - using SdkVersion = std::string; - alignas(SdkVersion) static unsigned char storage[sizeof(SdkVersion)]; - static const SdkVersion* sdkVersion = ::new (storage) - SdkVersion(EVTSDK_VERSION_PREFIX "-" OS_NAME "-C++-" ECS_SUPP "-" BUILD_VERSION_STR); - return *sdkVersion; + // A literal survives late workers without leaking a string buffer on library unload. + return EVTSDK_VERSION_PREFIX "-" OS_NAME "-C++-" ECS_SUPP "-" BUILD_VERSION_STR; } void PlatformAbstractionLayer::initialize(IRuntimeConfig& configuration) diff --git a/lib/pal/PAL.hpp b/lib/pal/PAL.hpp index f79cdd080..c18c458fd 100644 --- a/lib/pal/PAL.hpp +++ b/lib/pal/PAL.hpp @@ -78,7 +78,7 @@ namespace PAL_NS_BEGIN public: void sleep(unsigned delayMs) const noexcept; - const std::string& getSdkVersion() const; + const char* getSdkVersion() const; std::string generateUuidString() const; @@ -136,7 +136,7 @@ namespace PAL_NS_BEGIN /** * Return SDK version in format "----". */ - inline const std::string& getSdkVersion() + inline const char* getSdkVersion() { return GetPAL().getSdkVersion(); } diff --git a/tests/embedding/CMakeLists.txt b/tests/embedding/CMakeLists.txt index 9b1a4cc8f..792283f2f 100644 --- a/tests/embedding/CMakeLists.txt +++ b/tests/embedding/CMakeLists.txt @@ -5,7 +5,22 @@ set(CMAKE_CXX_STANDARD 11) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_EXTENSIONS OFF) +option(MATSDK_EMBEDDING_ADDRESS_SANITIZER + "Run embedding regressions with AddressSanitizer and leak detection" OFF) +if(MATSDK_EMBEDDING_ADDRESS_SANITIZER) + if(NOT CMAKE_SYSTEM_NAME STREQUAL "Linux" OR + NOT CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") + message(FATAL_ERROR "Embedding ASan/LSan coverage requires Linux with GCC or Clang") + endif() + add_compile_options(-fsanitize=address -fno-omit-frame-pointer) + add_link_options(-fsanitize=address) +endif() + set(BUILD_SHARED_LIBS OFF CACHE BOOL "") +if(MATSDK_EMBEDDING_ADDRESS_SANITIZER AND BUILD_SHARED_LIBS) + message(FATAL_ERROR "Embedding ASan/LSan coverage requires static SDK embedding") +endif() + set(MATSDK_BUILD_HEADERS ON CACHE BOOL "" FORCE) set(MATSDK_BUILD_LIBRARY ON CACHE BOOL "" FORCE) set(MATSDK_BUILD_TEST_TOOL OFF CACHE BOOL "" FORCE) @@ -115,6 +130,10 @@ endif() enable_testing() add_test(NAME embedding_test COMMAND embedding_test) include("${CMAKE_CURRENT_LIST_DIR}/lifetime-tests.cmake") +if(MATSDK_EMBEDDING_ADDRESS_SANITIZER) + set_tests_properties(sdk_version_reload PROPERTIES + ENVIRONMENT "ASAN_OPTIONS=detect_leaks=1;LSAN_OPTIONS=exitcode=23") +endif() add_test(NAME apple_embedding_options COMMAND "${CMAKE_COMMAND}" "-DOUTPUT_DIR=${CMAKE_CURRENT_BINARY_DIR}/apple-options" diff --git a/tests/embedding/lifetime-tests.cmake b/tests/embedding/lifetime-tests.cmake index 9db97aac4..8de708589 100644 --- a/tests/embedding/lifetime-tests.cmake +++ b/tests/embedding/lifetime-tests.cmake @@ -14,6 +14,14 @@ foreach(source pal/PAL.cpp api/AuthTokensController.cpp) instrumented "${instrumented}") string(REPLACE "using SdkVersion = std::string;" "using SdkVersion = LifetimeString;" instrumented "${instrumented}") + # Hook entry, not a local variable: the allocation-free implementation has none. + string(REGEX REPLACE "(PlatformAbstractionLayer::getSdkVersion\\(\\) const[ \r\n]*\\{)" + "\\1\n LifetimeVersionAccessed();" instrumented "${instrumented}") + if(NOT instrumented MATCHES "LifetimeVersionAccessed\\(\\);") + message(FATAL_ERROR "Update version accessor lifetime instrumentation") + endif() + string(PREPEND instrumented + "extern \"C\" void LifetimeVersionAccessed();\n") set(hook "LifetimeVersionDestroyed") set(type String) set(base string) @@ -26,7 +34,7 @@ foreach(source pal/PAL.cpp api/AuthTokensController.cpp) set(type Mutex) set(base mutex) endif() - if(NOT instrumented MATCHES "(static const LifetimeString sdkVersion|using SdkVersion = LifetimeString|static LifetimeMutex mutex|using AuthMutex = LifetimeMutex)") + if(NOT instrumented MATCHES "(LifetimeVersionAccessed\\(\\);|static LifetimeMutex mutex|using AuthMutex = LifetimeMutex)") message(FATAL_ERROR "Update lifetime instrumentation for ${source}") endif() string(PREPEND instrumented @@ -43,6 +51,7 @@ endforeach() add_executable(sdk_lifetime_tests "${CMAKE_CURRENT_LIST_DIR}/lifetime-tests.cpp" "${lifetime_dir}/PAL.cpp" "${lifetime_dir}/AuthTokensController.cpp") target_link_libraries(sdk_lifetime_tests PRIVATE MSTelemetry::mat matsdk_internal_config matsdk_build_options) +target_compile_features(sdk_lifetime_tests PRIVATE cxx_std_17) target_include_directories(sdk_lifetime_tests PRIVATE "$" "${sdk_source}/lib/api") add_dependencies(embedding_test sdk_lifetime_tests) @@ -50,3 +59,27 @@ foreach(mode version mutex both) add_test(NAME sdk_lifetime_${mode} COMMAND sdk_lifetime_tests "${mode}") set_tests_properties(sdk_lifetime_${mode} PROPERTIES TIMEOUT 30) endforeach() + +# Embed the actual archive, not the instrumented copies, to catch unload-time leaks. +add_library(sdk_version_reload_fixture SHARED version-reload-fixture.cpp) +target_link_libraries(sdk_version_reload_fixture PRIVATE MSTelemetry::mat + matsdk_internal_config matsdk_build_options) +target_compile_features(sdk_version_reload_fixture PRIVATE cxx_std_17) +target_include_directories(sdk_version_reload_fixture PRIVATE + "$") +if(CMAKE_SYSTEM_NAME STREQUAL "Linux") + # GNU-unique symbols pin DSOs; symbol interposition can bind to another SDK copy. + target_compile_options(mat PRIVATE $<$:-fno-gnu-unique>) + target_link_options(sdk_version_reload_fixture PRIVATE + "-Wl,--exclude-libs,ALL" "-Wl,-Bsymbolic") +endif() + +add_executable(sdk_version_reload_test version-reload-test.cpp) +target_link_libraries(sdk_version_reload_test PRIVATE ${CMAKE_DL_LIBS} + matsdk_internal_config matsdk_build_options) +target_include_directories(sdk_version_reload_test PRIVATE + "$") +add_dependencies(embedding_test sdk_version_reload_fixture sdk_version_reload_test) +add_test(NAME sdk_version_reload + COMMAND sdk_version_reload_test "$") +set_tests_properties(sdk_version_reload PROPERTIES TIMEOUT 30) diff --git a/tests/embedding/lifetime-tests.cpp b/tests/embedding/lifetime-tests.cpp index 90df11436..be59e18c8 100644 --- a/tests/embedding/lifetime-tests.cpp +++ b/tests/embedding/lifetime-tests.cpp @@ -2,6 +2,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // SPDX-License-Identifier: Apache-2.0 // +#include "Version.hpp" #include "api/AuthTokensController.hpp" #include "pal/PAL.hpp" @@ -10,6 +11,7 @@ #include #include #include +#include #include namespace @@ -18,7 +20,7 @@ namespace std::atomic mutexDestroyed{false}; bool checkVersion = false; bool checkMutex = false; - const std::string* versionAddress = nullptr; + const char* versionAddress = nullptr; std::mutex* mutexAddress = nullptr; void Fail(const char* message) @@ -39,24 +41,34 @@ namespace { Fail("SDK version was destroyed before its exit owner"); } - const auto& version = PAL::GetPAL().getSdkVersion(); - if (version.empty() || (versionAddress && versionAddress != &version)) + + const std::string_view version = PAL::GetPAL().getSdkVersion(); + const std::string_view prefix = EVTSDK_VERSION_PREFIX "-"; + const std::string_view suffix = "-C++-" ECS_SUPP "-" BUILD_VERSION_STR; + if (version.substr(0, prefix.size()) != prefix || + version.size() < suffix.size() || + version.substr(version.size() - suffix.size()) != suffix || + (versionAddress && versionAddress != version.data())) { Fail("SDK version did not retain its value and identity"); } - versionAddress = &version; + + versionAddress = version.data(); } + if (checkMutex) { if (mutexDestroyed.load()) { Fail("Auth mutex was destroyed before its exit owner"); } + auto& mutex = MAT::AuthTokensController::GetMutex(); if (mutexAddress && mutexAddress != &mutex) { Fail("Auth mutex did not retain its identity"); } + std::lock_guard lock(mutex); mutexAddress = &mutex; } @@ -75,6 +87,14 @@ extern "C" void LifetimeVersionDestroyed() versionDestroyed.store(true); } +extern "C" void LifetimeVersionAccessed() +{ + if (versionDestroyed.load()) + { + Fail("SDK version accessed after destruction"); + } +} + extern "C" void LifetimeMutexDestroyed() { mutexDestroyed.store(true); @@ -86,12 +106,14 @@ int main(int argc, char** argv) { return 2; } + checkVersion = std::strcmp(argv[1], "version") == 0 || std::strcmp(argv[1], "both") == 0; checkMutex = std::strcmp(argv[1], "mutex") == 0 || std::strcmp(argv[1], "both") == 0; if ((!checkVersion && !checkMutex) || std::atexit(ExitOwner) != 0) { return 2; } + ReadState(); ReadState(); return 0; diff --git a/tests/embedding/version-reload-fixture.cpp b/tests/embedding/version-reload-fixture.cpp new file mode 100644 index 000000000..8f63cd91c --- /dev/null +++ b/tests/embedding/version-reload-fixture.cpp @@ -0,0 +1,18 @@ +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +#include "pal/PAL.hpp" + +#include + +#ifdef _WIN32 +#define EXPORTED __declspec(dllexport) +#else +#define EXPORTED __attribute__((visibility("default"))) +#endif + +extern "C" EXPORTED const char* GetSdkVersionForReload() +{ + return std::string_view(PAL::getSdkVersion()).data(); +} diff --git a/tests/embedding/version-reload-test.cpp b/tests/embedding/version-reload-test.cpp new file mode 100644 index 000000000..d07fc5785 --- /dev/null +++ b/tests/embedding/version-reload-test.cpp @@ -0,0 +1,96 @@ +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +#include "Version.hpp" +#include "mat/config.h" + +#include +#include + +#ifdef _WIN32 +#include +#else +#include +#endif + +namespace +{ + bool CheckVersion(const char* version) + { + const char prefix[] = EVTSDK_VERSION_PREFIX "-"; +#ifdef HAVE_MAT_EXP + const char suffix[] = "-C++-ECS-" BUILD_VERSION_STR; +#else + const char suffix[] = "-C++-No-" BUILD_VERSION_STR; +#endif + if (!version) + { + return false; + } + + const auto size = std::strlen(version); + return size >= sizeof(suffix) - 1 && + std::strncmp(version, prefix, sizeof(prefix) - 1) == 0 && + std::strcmp(version + size - (sizeof(suffix) - 1), suffix) == 0; + } + + bool Reload(const char* path) + { +#ifdef _WIN32 + const auto library = ::LoadLibraryA(path); + const auto version = library + ? reinterpret_cast(::GetProcAddress(library, "GetSdkVersionForReload")) + : nullptr; +#else + auto* library = dlopen(path, RTLD_NOW | RTLD_LOCAL); + const auto version = library + ? reinterpret_cast(dlsym(library, "GetSdkVersionForReload")) + : nullptr; +#endif + if (!library) + { + std::fprintf(stderr, "Unable to load SDK fixture: %s\n", path); + return false; + } + + // Do not keep a pointer into an unloaded DSO or heap buffer on the stack. + const bool valid = version && CheckVersion(version()) && CheckVersion(version()); +#ifdef _WIN32 + const bool unloaded = ::FreeLibrary(library) && !::GetModuleHandleA(path); +#else + const bool closed = dlclose(library) == 0; + auto* remaining = dlopen(path, RTLD_NOW | RTLD_NOLOAD); + const bool unloaded = closed && !remaining; + if (remaining) + { + dlclose(remaining); + } +#endif + if (!valid || !unloaded) + { + std::fprintf(stderr, "SDK version validation or actual library unload failed\n"); + } + + return valid && unloaded; + } +} + +int main(int argc, char** argv) +{ + if (argc != 2) + { + return 2; + } + + for (int cycle = 0; cycle != 2; ++cycle) + { + if (!Reload(argv[1])) + { + return 1; + } + } + + std::puts("PASS two SDK version unload/reload cycles"); + return 0; +}