diff --git a/.github/workflows/build-android.yml b/.github/workflows/build-android.yml index 06bce4267..d25e81e72 100644 --- a/.github/workflows/build-android.yml +++ b/.github/workflows/build-android.yml @@ -17,13 +17,18 @@ on: - main - dev + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: build: runs-on: windows-latest name: Build for Android steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: submodules: false - name: Update submodules @@ -32,7 +37,7 @@ jobs: git config --global submodule.lib/modules.update none git -c protocol.version=2 submodule update --init --force --depth=1 - name: Setup Java - uses: actions/setup-java@v3 + uses: actions/setup-java@v5 with: distribution: 'adopt' java-version: '17' @@ -40,14 +45,13 @@ jobs: # Workaround for: 'Unable to decrypt local Maven settings credentials' run: rm $Env:USERPROFILE\.m2\settings.xml - name: Setup Android SDK - uses: android-actions/setup-android@v2 + uses: android-actions/setup-android@v3 - name: Install NDK run: | java -version gci env:* | sort-object name - new-item "C:\Users\runneradmin\.android\repositories.cfg" -ItemType "file" - echo yes | .\sdkmanager.bat "ndk-bundle" "cmake;3.10.2.4988404" "ndk;27.0.12077973" --sdk_root=$Env:ANDROID_SDK_ROOT - working-directory: ${{ env.ANDROID_SDK_ROOT }}\cmdline-tools\7.0\bin + new-item "$Env:USERPROFILE\.android\repositories.cfg" -ItemType "file" + echo yes | sdkmanager "ndk-bundle" "cmake;3.10.2.4988404" "ndk;27.0.12077973" --sdk_root=$Env:ANDROID_SDK_ROOT - name: Chocolatey run: | choco install --no-progress -y ninja diff --git a/.github/workflows/build-ios-mac.yml b/.github/workflows/build-ios-mac.yml index 80c5f981d..2ed1ac867 100644 --- a/.github/workflows/build-ios-mac.yml +++ b/.github/workflows/build-ios-mac.yml @@ -19,17 +19,22 @@ on: schedule: - cron: 0 2 * * 1-5 + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: build: strategy: matrix: - os: [macos-13, macos-15] + os: [macos-14, macos-15] config: [release, debug] simulator: ["'iPhone 15'", "'iPad Pro (11-inch) (4th generation)'", "'iPhone 16'", "'iPad Air 11-inch (M2)'"] exclude: - - os: macos-13 + - os: macos-14 simulator: "'iPhone 16'" - - os: macos-13 + - os: macos-14 simulator: "'iPad Air 11-inch (M2)'" - os: macos-15 simulator: "'iPhone 15'" @@ -42,14 +47,14 @@ jobs: - name: Grant write permissions to /usr/local run: | sudo chown -R $USER:staff /usr/local - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 with: submodules: 'true' continue-on-error: true - name: build run: | - if [[ "${{ matrix.os }}" == "macos-13" ]]; then - export IOS_DEPLOYMENT_TARGET=13.0; + if [[ "${{ matrix.os }}" == "macos-14" ]]; then + export IOS_DEPLOYMENT_TARGET=14.0; elif [[ "${{ matrix.os }}" == "macos-15" ]]; then export IOS_DEPLOYMENT_TARGET=15.0; fi diff --git a/.github/workflows/build-posix-latest.yml b/.github/workflows/build-posix-latest.yml index 2271a459b..9990c6bb2 100644 --- a/.github/workflows/build-posix-latest.yml +++ b/.github/workflows/build-posix-latest.yml @@ -19,6 +19,11 @@ on: schedule: - cron: 0 2 * * 1-5 + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: build: @@ -32,7 +37,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v1 + uses: actions/checkout@v4 continue-on-error: true - name: Test ${{ matrix.os }} ${{ matrix.config }} run: ./build-tests.sh ${{ matrix.config }} diff --git a/.github/workflows/build-ubuntu-2204.yml b/.github/workflows/build-ubuntu-2204.yml index e8f456bbc..4a74cf84f 100644 --- a/.github/workflows/build-ubuntu-2204.yml +++ b/.github/workflows/build-ubuntu-2204.yml @@ -19,6 +19,11 @@ on: schedule: - cron: 0 2 * * 1-5 + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: build: @@ -32,7 +37,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v1 + uses: actions/checkout@v4 continue-on-error: true - name: Test ${{ matrix.os }} ${{ matrix.config }} run: ./build-tests.sh ${{ matrix.config }} \ No newline at end of file diff --git a/.github/workflows/build-windows-clang.yaml.off b/.github/workflows/build-windows-clang.yaml.off index 9f2c790ea..2621613c1 100644 --- a/.github/workflows/build-windows-clang.yaml.off +++ b/.github/workflows/build-windows-clang.yaml.off @@ -22,7 +22,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v4 continue-on-error: true - name: Setup Tools diff --git a/.github/workflows/build-windows-vs2017.yaml.off b/.github/workflows/build-windows-vs2017.yaml.off index 1db4607db..a2d22827b 100644 --- a/.github/workflows/build-windows-vs2017.yaml.off +++ b/.github/workflows/build-windows-vs2017.yaml.off @@ -22,7 +22,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v4 continue-on-error: true - name: Setup Tools diff --git a/.github/workflows/build-windows-vs2022.yaml b/.github/workflows/build-windows-vs2022.yaml index a8a18394e..20605b442 100644 --- a/.github/workflows/build-windows-vs2022.yaml +++ b/.github/workflows/build-windows-vs2022.yaml @@ -22,7 +22,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v4 continue-on-error: true - name: Build diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 503cae673..d0a392690 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -14,6 +14,11 @@ on: schedule: - cron: '0 8 * * 1' + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: analyze: name: Analyze @@ -34,7 +39,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v4 continue-on-error: true # Initializes the CodeQL tools for scanning. @@ -87,7 +92,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v4 continue-on-error: true - name: Update submodules @@ -102,21 +107,20 @@ jobs: languages: java - name: Setup Java - uses: actions/setup-java@v3 + uses: actions/setup-java@v5 with: distribution: 'adopt' java-version: '17' - name: Remove default github maven configuration run: rm $Env:USERPROFILE\.m2\settings.xml - name: Setup Android SDK - uses: android-actions/setup-android@v2 + uses: android-actions/setup-android@v3 - name: Install NDK run: | java -version gci env:* | sort-object name - new-item "C:\Users\runneradmin\.android\repositories.cfg" -ItemType "file" - echo yes | .\sdkmanager.bat "ndk-bundle" "cmake;3.10.2.4988404" "ndk;27.0.12077973" --sdk_root=$Env:ANDROID_SDK_ROOT - working-directory: ${{ env.ANDROID_SDK_ROOT }}\cmdline-tools\7.0\bin + new-item "$Env:USERPROFILE\.android\repositories.cfg" -ItemType "file" + echo yes | sdkmanager "ndk-bundle" "cmake;3.10.2.4988404" "ndk;27.0.12077973" --sdk_root=$Env:ANDROID_SDK_ROOT - name: Chocolatey run: | choco install --no-progress -y ninja diff --git a/.github/workflows/spellcheck.yml b/.github/workflows/spellcheck.yml index 69b136fa1..f7e594992 100644 --- a/.github/workflows/spellcheck.yml +++ b/.github/workflows/spellcheck.yml @@ -6,6 +6,11 @@ on: pull_request: branches: [ master, main ] + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: build: runs-on: ubuntu-latest @@ -13,7 +18,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v4 continue-on-error: true - name: install misspell diff --git a/.github/workflows/test-android-mac.yml.off b/.github/workflows/test-android-mac.yml.off index 56b87579a..96e17c4a5 100644 --- a/.github/workflows/test-android-mac.yml.off +++ b/.github/workflows/test-android-mac.yml.off @@ -26,7 +26,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v4 with: submodules: true depth: 1 @@ -42,7 +42,7 @@ jobs: script: ./testandlog - name: Upload if: ${{ always() }} - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: logcat path: ./lib/android_build/logcat.txt \ No newline at end of file diff --git a/.github/workflows/test-win-latest.yml b/.github/workflows/test-win-latest.yml index 760c88fb0..0fad50e9f 100644 --- a/.github/workflows/test-win-latest.yml +++ b/.github/workflows/test-win-latest.yml @@ -19,6 +19,11 @@ on: schedule: - cron: 0 2 * * 1-5 + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: test: name: Test on Windows ${{ matrix.arch }}-${{ matrix.build }} @@ -32,13 +37,13 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v1 + uses: actions/checkout@v4 continue-on-error: true - name: setup-msbuild - uses: microsoft/setup-msbuild@v1.1 + uses: microsoft/setup-msbuild@v2 with: - vs-version: '[16,)' + vs-version: '[17,)' - name: Test ${{ matrix.arch }} ${{ matrix.build }} shell: cmd diff --git a/CMakeLists.txt b/CMakeLists.txt index 959134a61..9fcd978a1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -115,23 +115,30 @@ message("-- CMAKE_CXX_COMPILER_ID: ${CMAKE_CXX_COMPILER_ID}") include(tools/ParseOsRelease.cmake) if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC") -set(WARN_FLAGS "/W4 /WX") + set(WARN_FLAGS "/W4 /WX") +elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") + # -Wno-unknown-warning-option is Clang-only, omitted here + set(WARN_FLAGS "-Wall -Werror -Wextra -Wno-unused-parameter -Wno-unused-but-set-variable") + # -Wno-reorder is C++-only; added to CXX_FLAGS below (suppresses member-init-order warnings in submodule code) + set(CXX_EXTRA_WARN_FLAGS "-Wno-reorder") else() -# No -pedantic -Wno-extra-semi -Wno-gnu-zero-variadic-macro-arguments -set(WARN_FLAGS "-Wall -Werror -Wextra -Wno-unused-parameter -Wno-unknown-warning-option -Wno-unused-but-set-variable") + # Clang / AppleClang + set(WARN_FLAGS "-Wall -Werror -Wextra -Wno-unused-parameter -Wno-unknown-warning-option -Wno-unused-but-set-variable") + # -Wno-reorder is C++-only; added to CXX_FLAGS below (suppresses member-init-order warnings in submodule code) + set(CXX_EXTRA_WARN_FLAGS "-Wno-reorder") endif() if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") # Using GCC with -s and -Wl linker flags - set(REL_FLAGS "-s -Wl,--gc-sections -Os ${WARN_FLAGS} -ffunction-sections -fdata-sections -fmerge-all-constants -ffast-math") + set(REL_FLAGS "-s -Wl,--gc-sections -Os ${WARN_FLAGS} -ffunction-sections -fdata-sections -fmerge-all-constants -ffast-math -fno-finite-math-only") elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC") set(REL_FLAGS "${WARN_FLAGS}") elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang") # AppleClang does not support -ffunction-sections and -fdata-sections with the -fembed-bitcode and -fembed-bitcode-marker - set(REL_FLAGS "-Os ${WARN_FLAGS} -fmerge-all-constants -ffast-math") + set(REL_FLAGS "-Os ${WARN_FLAGS} -fmerge-all-constants -ffast-math -fno-finite-math-only") else() # Using clang - strip unsupported GCC options - set(REL_FLAGS "-Os ${WARN_FLAGS} -ffunction-sections -fmerge-all-constants -ffast-math") + set(REL_FLAGS "-Os ${WARN_FLAGS} -ffunction-sections -fmerge-all-constants -ffast-math -fno-finite-math-only") endif() ## Uncomment this to reduce the volume of note warnings on RPi4 w/gcc-8 Ref. https://gcc.gnu.org/ml/gcc/2017-05/msg00073.html @@ -146,13 +153,13 @@ if (NOT CMAKE_BUILD_TYPE STREQUAL "Debug") #TODO: -fno-rtti message("Building Release ...") set(CMAKE_C_FLAGS "$ENV{CFLAGS} ${CMAKE_C_FLAGS} -std=c11 ${REL_FLAGS}") - set(CMAKE_CXX_FLAGS "$ENV{CXXFLAGS} ${CMAKE_CXX_FLAGS} -std=c++11 ${REL_FLAGS}") + set(CMAKE_CXX_FLAGS "$ENV{CXXFLAGS} ${CMAKE_CXX_FLAGS} -std=c++11 ${REL_FLAGS} ${CXX_EXTRA_WARN_FLAGS}") else() set(USE_TCMALLOC 1) message("Building Debug ...") include(tools/FindTcmalloc.cmake) set(CMAKE_C_FLAGS "$ENV{CFLAGS} ${CMAKE_C_FLAGS} -std=c11 ${DBG_FLAGS}") - set(CMAKE_CXX_FLAGS "$ENV{CXXFLAGS} ${CMAKE_CXX_FLAGS} -std=c++11 ${DBG_FLAGS}") + set(CMAKE_CXX_FLAGS "$ENV{CXXFLAGS} ${CMAKE_CXX_FLAGS} -std=c++11 ${DBG_FLAGS} ${CXX_EXTRA_WARN_FLAGS}") endif() #Remove /Zi for Win32 debug compiler issue diff --git a/build-tests-ios.sh b/build-tests-ios.sh index bf29a50b3..86928267a 100755 --- a/build-tests-ios.sh +++ b/build-tests-ios.sh @@ -7,14 +7,23 @@ set -e ./build-ios.sh ${SKU} -# dyld_info /Users/runner/work/cpp_client_telemetry/cpp_client_telemetry/out/lib/libmat.a - cd tests/unittests xcrun simctl list devices available echo 'End of xcrun simctl list devices available' -xcodebuild test -scheme iOSUnitTests -destination "platform=iOS Simulator,name=$SIMULATOR" +# Resolve simulator UUID to avoid ambiguous destination matching. +# Grab the last (newest OS) available device matching the requested name. +SIM_ID=$(xcrun simctl list devices available | grep "$SIMULATOR" | grep -oE '[A-F0-9-]{36}' | tail -1) + +if [ -z "$SIM_ID" ]; then + echo "ERROR: No available simulator found for '$SIMULATOR'" + exit 1 +fi + +echo "Using simulator: $SIMULATOR (id=$SIM_ID)" + +xcodebuild test -scheme iOSUnitTests -destination "id=$SIM_ID" cd ../functests -xcodebuild test -scheme iOSFuncTests -destination "platform=iOS Simulator,name=$SIMULATOR" +xcodebuild test -scheme iOSFuncTests -destination "id=$SIM_ID" diff --git a/lib/api/Logger.cpp b/lib/api/Logger.cpp index 54d883664..f76f85734 100644 --- a/lib/api/Logger.cpp +++ b/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 diff --git a/lib/http/HttpClientFactory.cpp b/lib/http/HttpClientFactory.cpp index 5419f161d..9424c853a 100644 --- a/lib/http/HttpClientFactory.cpp +++ b/lib/http/HttpClientFactory.cpp @@ -22,9 +22,9 @@ #elif defined(MATSDK_PAL_CPP11) #if TARGET_OS_IPHONE || (defined(__APPLE__) && defined(APPLE_HTTP)) #include "http/HttpClient_Apple.hpp" - #elif defined(HAVE_MAT_CURL_HTTP_CLIENT) || !defined(ANDROID) + #elif defined(HAVE_MAT_CURL_HTTP_CLIENT) || !defined(__ANDROID__) #include "http/HttpClient_Curl.hpp" - #elif defined(ANDROID) + #elif defined(__ANDROID__) #include "http/HttpClient_Android.hpp" #endif #else @@ -61,7 +61,7 @@ namespace MAT_NS_BEGIN { LOG_TRACE("Creating HttpClient_Apple"); return std::make_shared(); } -#elif defined(ANDROID) +#elif defined(__ANDROID__) std::shared_ptr HttpClientFactory::Create() { LOG_TRACE("Creating HttpClient_Android"); return HttpClient_Android::GetClientInstance(); diff --git a/lib/http/HttpClientManager.cpp b/lib/http/HttpClientManager.cpp index 58fa5fb4a..a1c228556 100644 --- a/lib/http/HttpClientManager.cpp +++ b/lib/http/HttpClientManager.cpp @@ -149,8 +149,15 @@ namespace MAT_NS_BEGIN { void HttpClientManager::cancelAllRequests() { cancelAllRequestsAsync(); - while (!m_httpCallbacks.empty()) + while (true) + { + { + LOCKGUARD(m_httpCallbacksMtx); + if (m_httpCallbacks.empty()) + break; + } std::this_thread::yield(); + } } // start async cancellation diff --git a/lib/http/HttpClient_Android.cpp b/lib/http/HttpClient_Android.cpp index dbbe0eee1..1410ad399 100644 --- a/lib/http/HttpClient_Android.cpp +++ b/lib/http/HttpClient_Android.cpp @@ -11,7 +11,8 @@ namespace MAT_NS_BEGIN { - constexpr static auto Tag = "HttpClient_Android"; + // Log tag for __android_log_print (reserved for future diagnostics) + constexpr static auto Tag __attribute__((unused)) = "HttpClient_Android"; HttpClient_Android::HttpRequest::~HttpRequest() noexcept { diff --git a/lib/http/HttpClient_Apple.hpp b/lib/http/HttpClient_Apple.hpp index e9b22b1f0..1eee21b97 100644 --- a/lib/http/HttpClient_Apple.hpp +++ b/lib/http/HttpClient_Apple.hpp @@ -25,10 +25,13 @@ namespace MAT_NS_BEGIN { void Erase(IHttpRequest* req); void Add(IHttpRequest* req); + void* GetOrCreateSession(); private: + void ensureSession(); std::mutex m_requestsMtx; std::map m_requests; + void* m_session = nullptr; // NSURLSession*, opaque in header }; } MAT_NS_END diff --git a/lib/http/HttpClient_Apple.mm b/lib/http/HttpClient_Apple.mm index 05817087a..7b075f472 100644 --- a/lib/http/HttpClient_Apple.mm +++ b/lib/http/HttpClient_Apple.mm @@ -29,9 +29,6 @@ return std::string("RESP-") + std::to_string(seq.fetch_add(1)); } -static dispatch_once_t once; -static NSURLSession* session; - class HttpRequestApple : public SimpleHttpRequest { public: @@ -40,10 +37,6 @@ m_parent(parent) { m_parent->Add(static_cast(this)); - dispatch_once(&once, ^{ - NSURLSessionConfiguration* sessionConfig = [NSURLSessionConfiguration defaultSessionConfiguration]; - session = [NSURLSession sessionWithConfiguration:sessionConfig]; - }); } ~HttpRequestApple() noexcept @@ -72,16 +65,18 @@ void SendAsync(IHttpResponseCallback* callback) HandleResponse(data, response, error); }; + NSURLSession* sess = (__bridge NSURLSession*)m_parent->GetOrCreateSession(); + if(equalsIgnoreCase(m_method, "get")) { [m_urlRequest setHTTPMethod:@"GET"]; - m_dataTask = [session dataTaskWithRequest:m_urlRequest completionHandler:m_completionMethod]; + m_dataTask = [sess dataTaskWithRequest:m_urlRequest completionHandler:m_completionMethod]; } else { [m_urlRequest setHTTPMethod:@"POST"]; NSData* postData = [NSData dataWithBytes:m_body.data() length:m_body.size()]; - m_dataTask = [session uploadTaskWithRequest:m_urlRequest fromData:postData completionHandler:m_completionMethod]; + m_dataTask = [sess uploadTaskWithRequest:m_urlRequest fromData:postData completionHandler:m_completionMethod]; } [m_dataTask resume]; @@ -132,23 +127,6 @@ void HandleResponse(NSData* data, NSURLResponse* response, NSError* error) void Cancel() { [m_dataTask cancel]; - [session getTasksWithCompletionHandler:^(NSArray* dataTasks, NSArray* uploadTasks, NSArray* downloadTasks) - { - for (NSURLSessionTask* _task in dataTasks) - { - [_task cancel]; - } - - for (NSURLSessionTask* _task in downloadTasks) - { - [_task cancel]; - } - - for (NSURLSessionTask* _task in uploadTasks) - { - [_task cancel]; - } - }]; } private: @@ -161,14 +139,38 @@ void Cancel() HttpClient_Apple::HttpClient_Apple() { + ensureSession(); LOG_TRACE("Initializing HttpClient_Apple..."); } HttpClient_Apple::~HttpClient_Apple() noexcept { + // Release the session object. By this point, CancelAllRequests should + // have already invalidated the session and drained all tasks. + if (m_session) { + NSURLSession* sess = (__bridge_transfer NSURLSession*)m_session; + [sess invalidateAndCancel]; + m_session = nullptr; + } LOG_TRACE("Shutting down HttpClient_Apple..."); } +void HttpClient_Apple::ensureSession() +{ + if (!m_session) { + @autoreleasepool { + NSURLSessionConfiguration* sessionConfig = [NSURLSessionConfiguration ephemeralSessionConfiguration]; + m_session = (__bridge_retained void*)[NSURLSession sessionWithConfiguration:sessionConfig]; + } + } +} + +void* HttpClient_Apple::GetOrCreateSession() +{ + ensureSession(); + return m_session; +} + IHttpRequest* HttpClient_Apple::CreateRequest() { auto request = new HttpRequestApple(this); @@ -219,6 +221,15 @@ void Cancel() PAL::sleep(100); std::this_thread::yield(); } + + // Invalidate the session so no stale completion handlers can fire after + // the caller (HttpClientManager) finishes tearing down. A fresh session + // is created lazily on the next CreateRequest if the SDK is reinitialized. + if (m_session) { + NSURLSession* sess = (__bridge_transfer NSURLSession*)m_session; + [sess invalidateAndCancel]; + m_session = nullptr; + } } void HttpClient_Apple::Erase(IHttpRequest* req) diff --git a/lib/http/HttpResponseDecoder.cpp b/lib/http/HttpResponseDecoder.cpp index 11e9d4096..2bb652fdf 100644 --- a/lib/http/HttpResponseDecoder.cpp +++ b/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; } @@ -129,6 +127,7 @@ namespace MAT_NS_BEGIN { evt.param1 = 0; // response.GetStatusCode(); DispatchEvent(evt); } + delete ctx->httpResponse; ctx->httpResponse = nullptr; // eventsRejected(ctx); // FIXME: [MG] - investigate why ctx gets corrupt after eventsRejected requestAborted(ctx); @@ -159,6 +158,8 @@ namespace MAT_NS_BEGIN { evt.param1 = response.GetStatusCode(); DispatchEvent(evt); } + delete ctx->httpResponse; + ctx->httpResponse = nullptr; temporaryNetworkFailure(ctx); break; } diff --git a/lib/include/mat/config-default.h b/lib/include/mat/config-default.h index 9617611c9..e5ce48e49 100644 --- a/lib/include/mat/config-default.h +++ b/lib/include/mat/config-default.h @@ -8,16 +8,24 @@ #if defined(_WIN32) #if defined __has_include # if __has_include ("modules/azmon/AITelemetrySystem.hpp") -# define HAVE_MAT_AI +# ifndef HAVE_MAT_AI +# define HAVE_MAT_AI +# endif # endif # if __has_include ("modules/utc/UtcTelemetrySystem.hpp") -# define HAVE_MAT_UTC +# ifndef HAVE_MAT_UTC +# define HAVE_MAT_UTC +# endif # endif # if __has_include("modules/signals/Signals.hpp") -# define HAVE_MAT_SIGNALS +# ifndef HAVE_MAT_SIGNALS +# define HAVE_MAT_SIGNALS +# endif # endif # if __has_include("modules/sanitizer/Sanitizer.hpp") -# define HAVE_MAT_SANITIZER +# ifndef HAVE_MAT_SANITIZER +# define HAVE_MAT_SANITIZER +# endif # endif #endif #endif diff --git a/lib/pal/WorkerThread.cpp b/lib/pal/WorkerThread.cpp index 2bdbf6c67..8a9c21f97 100644 --- a/lib/pal/WorkerThread.cpp +++ b/lib/pal/WorkerThread.cpp @@ -56,22 +56,26 @@ namespace PAL_NS_BEGIN { auto item = new WorkerThreadShutdownItem(); Queue(item); std::thread::id this_id = std::this_thread::get_id(); + bool joined = false; try { - if (m_hThread.joinable() && (m_hThread.get_id() != this_id)) + if (m_hThread.joinable() && (m_hThread.get_id() != this_id)) { m_hThread.join(); - else + joined = true; + } else { m_hThread.detach(); + } } catch (...) {}; - // TODO: [MG] - investigate if we ever drop work items on shutdown. - if (!m_queue.empty()) - { - LOG_WARN("m_queue is not empty!"); - } - if (!m_timerQueue.empty()) - { - LOG_WARN("m_timerQueue is not empty!"); + // Clean up any tasks remaining in the queues after shutdown. + // Only safe after join() — the thread has fully exited. + // After detach(), the thread still needs the shutdown item + // and may still be accessing the queues. + if (joined) { + for (auto task : m_queue) { delete task; } + m_queue.clear(); + for (auto task : m_timerQueue) { delete task; } + m_timerQueue.clear(); } } diff --git a/lib/tpm/TransmissionPolicyManager.cpp b/lib/tpm/TransmissionPolicyManager.cpp index 83b82cf2a..3b79bb5d5 100644 --- a/lib/tpm/TransmissionPolicyManager.cpp +++ b/lib/tpm/TransmissionPolicyManager.cpp @@ -154,7 +154,7 @@ namespace MAT_NS_BEGIN { // m_isUploadScheduled check does not have to be strictly atomic because // the completion of upload will schedule more uploads as-needed, we only // want to avoid the unnecessary wasteful rescheduling. - LOG_TRACE("WAIT upload %d ms for lat=%d", delta, m_runningLatency); + LOG_TRACE("WAIT upload %d ms for lat=%d", delta, m_runningLatency.load()); return; } } @@ -173,7 +173,7 @@ namespace MAT_NS_BEGIN { { m_scheduledUploadTime = PAL::getMonotonicTimeMs() + delay.count(); m_runningLatency = latency; - LOG_TRACE("SCHED upload %d ms for lat=%d", delay.count(), m_runningLatency); + LOG_TRACE("SCHED upload %d ms for lat=%d", delay.count(), m_runningLatency.load()); m_scheduledUpload = PAL::scheduleTask(&m_taskDispatcher, static_cast(delay.count()), this, &TransmissionPolicyManager::uploadAsync, latency); } } @@ -185,10 +185,9 @@ namespace MAT_NS_BEGIN { return; } m_runningLatency = latency; - m_scheduledUploadTime = std::numeric_limits::max(); - { LOCKGUARD(m_scheduledUploadMutex); + m_scheduledUploadTime = std::numeric_limits::max(); m_isUploadScheduled = false; // Allow to schedule another uploadAsync if ((m_isPaused) || (m_scheduledUploadAborted)) { diff --git a/lib/tpm/TransmissionPolicyManager.hpp b/lib/tpm/TransmissionPolicyManager.hpp index e1a91ad10..003315a8a 100644 --- a/lib/tpm/TransmissionPolicyManager.hpp +++ b/lib/tpm/TransmissionPolicyManager.hpp @@ -131,7 +131,7 @@ constexpr const char* const DefaultBackoffConfig = "E,3000,300000,2,1"; size_t uploadCount() const noexcept; std::chrono::milliseconds m_timerdelay { std::chrono::seconds { 2 } }; - EventLatency m_runningLatency { EventLatency_RealTime }; + std::atomic m_runningLatency { EventLatency_RealTime }; TimerArray m_timers; public: diff --git a/tests/functests/AISendTests.cpp b/tests/functests/AISendTests.cpp index 180e9b074..31d4e9d38 100644 --- a/tests/functests/AISendTests.cpp +++ b/tests/functests/AISendTests.cpp @@ -118,7 +118,7 @@ class AISendTests : public ::testing::Test, } int port = server.addListeningPort(HTTP_PORT); std::ostringstream os; - os << "localhost:" << port; + os << "127.0.0.1:" << port; serverAddress = "http://" + os.str() + "/v2/track"; server.setServerName(os.str()); server.addHandler("/v2/track", *this); diff --git a/tests/functests/APITest.cpp b/tests/functests/APITest.cpp index 11524cd5a..4b6c38a15 100644 --- a/tests/functests/APITest.cpp +++ b/tests/functests/APITest.cpp @@ -391,6 +391,10 @@ TEST(APITest, LogManager_Initialize_DebugEventListener) LogManager::GetLogger()->LogEvent(eventToLog); } LogManager::Flush(); + // Storage-full callback fires asynchronously; give it time to arrive + for (int i = 0; i < 50 && debugListener.storageFullPct.load() < 100; i++) { + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } EXPECT_GE(debugListener.storageFullPct.load(), (unsigned)100); LogManager::FlushAndTeardown(); diff --git a/tests/functests/BasicFuncTests.cpp b/tests/functests/BasicFuncTests.cpp index 51848d054..f2133c5e8 100644 --- a/tests/functests/BasicFuncTests.cpp +++ b/tests/functests/BasicFuncTests.cpp @@ -154,7 +154,7 @@ class BasicFuncTests : public ::testing::Test, } int port = server.addListeningPort(HTTP_PORT); std::ostringstream os; - os << "localhost:" << port; + os << "127.0.0.1:" << port; serverAddress = "http://" + os.str() + "/simple/"; server.setServerName(os.str()); server.addHandler("/simple/", *this); @@ -257,15 +257,16 @@ class BasicFuncTests : public ::testing::Test, size_t lastIdx = 0; while ( ((PAL::getUtcSystemTimeMs()-start)<(1000* timeOutSec)) && (receivedEvents!=expected_count) ) { - /* Give time for our friendly HTTP server thread to process incoming request */ - std::this_thread::yield(); + /* Give time for HTTP server thread to process incoming request. + * sleep(10) instead of yield() reduces CPU contention on single-core + * iOS simulator runners and gives the network stack time to deliver. */ + PAL::sleep(10); { LOCKGUARD(mtx_requests); if (receivedRequests.size()) { size_t size = receivedRequests.size(); - //requests can come within 100 milisec sleep for (size_t index = lastIdx; index < size; index++) { auto request = receivedRequests.at(index); @@ -574,7 +575,7 @@ TEST_F(BasicFuncTests, sendNoPriorityEvents) logger->LogEvent(event2); LogManager::UploadNow(); - waitForEvents(1, 3); + waitForEvents(5, 3); EXPECT_GE(receivedRequests.size(), (size_t)1); LogManager::RemoveEventListener(EVT_HTTP_OK, listener); FlushAndTeardown(); @@ -673,7 +674,7 @@ TEST_F(BasicFuncTests, sendDifferentPriorityEvents) LogManager::UploadNow(); // 2 x customer events + 1 x evt_stats on start - waitForEvents(1, 3); + waitForEvents(5, 3); for (const auto &evt : { event, event2 }) { @@ -721,7 +722,7 @@ TEST_F(BasicFuncTests, sendMultipleTenantsTogether) LogManager::UploadNow(); // 2 x customer events + 1 x evt_stats on start - waitForEvents(1, 3); + waitForEvents(5, 3); for (const auto &evt : { event1, event2 }) { verifyEvent(evt, find(evt.GetName())); @@ -749,7 +750,7 @@ TEST_F(BasicFuncTests, configDecorations) logger->LogEvent(event4); LogManager::UploadNow(); - waitForEvents(2, 5); + waitForEvents(5, 5); for (const auto &evt : { event1, event2, event3, event4 }) { @@ -788,7 +789,7 @@ TEST_F(BasicFuncTests, restartRecoversEventsFromStorage) LogManager::UploadNow(); // 1st request for realtime event - waitForEvents(3, 5); // start, first_event, second_event, ongoing, stop, start, fooEvent + waitForEvents(10, 5); // start, first_event, second_event, ongoing, stop, start, fooEvent // we drop two of the events during pause, though. EXPECT_GE(receivedRequests.size(), (size_t)1); if (receivedRequests.size() != 0) @@ -852,7 +853,7 @@ TEST_F(BasicFuncTests, storageFileSizeDoesntExceedConfiguredSize) { Initialize(); - waitForEvents(2, 8); + waitForEvents(5, 8); if (receivedRequests.size()) { auto payload = decodeRequest(receivedRequests[0], false); @@ -898,7 +899,7 @@ TEST_F(BasicFuncTests, sendMetaStatsOnStart) Initialize(); LogManager::ResumeTransmission(); // ? LogManager::UploadNow(); - PAL::sleep(2000); + waitForEvents(5, 4); // (start + stop) + (2 events + start) auto r2 = records(); ASSERT_GE(r2.size(), (size_t)4); // (start + stop) + (2 events + start) @@ -928,7 +929,7 @@ TEST_F(BasicFuncTests, DiagLevelRequiredOnly_OneEventWithoutLevelOneWithButNotAl logger->LogEvent(eventWithAllowedLevel); LogManager::UploadNow(); - waitForEvents(1 /*timeout*/, 2 /*expected count*/); // Start and EventWithAllowedLevel + waitForEvents(5 /*timeout*/, 2 /*expected count*/); // Start and EventWithAllowedLevel ASSERT_EQ(records().size(), static_cast(2)); // Start and EventWithAllowedLevel @@ -971,7 +972,7 @@ TEST_F(BasicFuncTests, DiagLevelRequiredOnly_SendTwoEventsUpdateAllowedLevelsSen SendEventWithOptionalThenRequired(logger); LogManager::UploadNow(); - waitForEvents(2 /*timeout*/, 4 /*expected count*/); // Start and EventWithAllowedLevel + waitForEvents(5 /*timeout*/, 4 /*expected count*/); // Start and EventWithAllowedLevel auto sentRecords = records(); ASSERT_EQ(sentRecords.size(), static_cast(4)); // Start and EventWithAllowedLevel @@ -1173,9 +1174,9 @@ TEST_F(BasicFuncTests, killSwitchWorks) myLogger->LogEvent(event2); } } - // Try to upload and wait for 2 seconds to complete + // Try to upload and wait for completion LogManager::UploadNow(); - PAL::sleep(2000); + PAL::sleep(5000); // Log 100 events with valid logger LogManager::Initialize(TEST_TOKEN, configuration); diff --git a/tests/functests/MultipleLogManagersTests.cpp b/tests/functests/MultipleLogManagersTests.cpp index 25f99f175..e9210c5a7 100644 --- a/tests/functests/MultipleLogManagersTests.cpp +++ b/tests/functests/MultipleLogManagersTests.cpp @@ -74,7 +74,7 @@ class MultipleLogManagersTests : public ::testing::Test { int port = server.addListeningPort(0); std::ostringstream os; - os << "localhost:" << port; + os << "127.0.0.1:" << port; server.setServerName(os.str()); serverAddress = "http://" + os.str(); diff --git a/tests/unittests/HttpClientTests.cpp b/tests/unittests/HttpClientTests.cpp index 99d73248b..4b17bcce5 100644 --- a/tests/unittests/HttpClientTests.cpp +++ b/tests/unittests/HttpClientTests.cpp @@ -53,7 +53,7 @@ class HttpClientTests : public ::testing::Test, { _port = _server.addListeningPort(0); std::ostringstream os; - os << "localhost:" << _port; + os << "127.0.0.1:" << _port; _hostname = os.str(); _server.setServerName(_hostname); _server.addHandler("/simple/", *this); diff --git a/tests/unittests/PalTests.cpp b/tests/unittests/PalTests.cpp index 1ec078916..15bb98e5e 100644 --- a/tests/unittests/PalTests.cpp +++ b/tests/unittests/PalTests.cpp @@ -85,7 +85,7 @@ TEST_F(PalTests, SystemTime) int64_t t1 = PAL::getUtcSystemTimeMs(); EXPECT_THAT(t1, Gt(t0 + 360)); - EXPECT_THAT(t1, Lt(t0 + 550)); + EXPECT_THAT(t1, Lt(t0 + 1000)); } TEST_F(PalTests, FormatUtcTimestampMsAsISO8601) @@ -103,7 +103,7 @@ TEST_F(PalTests, MonotonicTime) int64_t t1 = PAL::getMonotonicTimeMs(); EXPECT_THAT(t1 - t0, Gt(780)); - EXPECT_THAT(t1 - t0, Lt(950)); + EXPECT_THAT(t1 - t0, Lt(1500)); } TEST_F(PalTests, SemanticContextPopulation)