Skip to content

Rearchitect vcpkg port for official submission#1414

Open
bmehta001 wants to merge 106 commits intomicrosoft:mainfrom
bmehta001:bhamehta/vcpkg-port-rearchitect
Open

Rearchitect vcpkg port for official submission#1414
bmehta001 wants to merge 106 commits intomicrosoft:mainfrom
bmehta001:bhamehta/vcpkg-port-rearchitect

Conversation

@bmehta001
Copy link

@bmehta001 bmehta001 commented Mar 16, 2026

Addresses #1412, #1390, and #781

  • Rearchitects the vcpkg port from a shell-script-based build with different behavior for different platforms, into a pure CMake workflow for official submission to the vcpkg registry that should works on all supported platforms
  • Have created vcpkg tests and a sample project where this library was imported into ONNXRuntime and successfully tested these and all existing unit/functional tests where applicable, on Windows, WSL, Android Simulator on WSL with NDK, macOS, and iOS Simulator on macOS

Changes:

vcpkg

  • For portfile.cmake, use vcpkg_from_github() + vcpkg_cmake_configure() + vcpkg_cmake_install() + vcpkg_cmake_config_fixup()
  • Added vcpkg.json manifest with dependency declarations (sqlite3, zlib, nlohmann-json, curl), platform constraints (!uwp), and optional feature flags (azmon, privacyguard, cds, signals, sanitizer, liveeventinspector).
    • zlib in vcpkg does not have the SIMD patches - have added more at the end for how to fix this.
  • Added usage file (vcpkg usage instructions shown after install)
  • Removed: CONTROL, TODO.md, get_repo_name.sh, response_file_linux.txt, response_file_mac.txt, v142-build.patch, since they are now redundant or outdated

CMake (had to modify to fix some behavior in legacy builds and for vcpkg)

  • Use MATSDK_USE_VCPKG_DEPS option to detect vcpkg toolchain and thus
    • Resolve dependencies with find_package() ; when OFF, use existing vendored/system-installed workflow
    • Skip manual compiler/platform flag configuration (architecture, paths, etc.) for vcpkg builds
    • Added CMake package config (cmake/MSTelemetryConfig.cmake.in) to generate find_package(MSTelemetry CONFIG) support with transitive dependency re-finding (find_dependency() for sqlite3, zlib, nlohmann-json, curl, pthreads as needed).
    • Added install/export targets — install(TARGETS mat EXPORT MSTelemetryTargets ...) with GNUInstallDirs, write_basic_package_version_file(), and configure_package_config_file() to specify paths in standardized way
    • Deduplicated and simplified dependency linking — replaced the tangled shared/static forking with a clear three-way branch: vcpkg mode (imported targets), Android legacy (bundled source compilation), and Linux/macOS legacy
      (-lsqlite3 -lz).
    • Added Android HTTP client selection (HttpClient_Android.cpp is now compiled on Android instead of HttpClient_Curl.cpp)
    • Link Apple frameworks like CoreFoundation, Foundation, CFNetwork, Network, SystemConfiguration, UIKit (iOS) / IOKit (macOS) via target_link_libraries() with -framework syntax.
    • Used message(STATUS ...) consistently throughout (or FATAL_ERROR) to use official CMake keywords
    • Quote EXISTS path arguments to prevent CMP0012 warnings.

Build flags

  • Added -fno-finite-math-only for Clang and AppleClang (helps fix issues for ORT builds consuming this library where -ffast-math is enabled), since sqlite3 uses the INFINITY macro, which is undefined under -ffinite-math-only
  • Move -Wno-reorder to apply only to CMAKE_CXX_FLAGS (since it's a C++-only flag)
  • Split GCC vs Clang warning flags (GCC doesn't support -Wno-unknown-warning-option)
  • Removed ZLIB_WINAPI from vcpkg builds since vcpkg's zlib does not use WINAPI/STDCALL.
  • Updated MATSDK_API_VERSION from 3.4 to 3.10 (which I assume is current SDK version)

Obj-C/Swift wrapper resilience

  • Added compile-time feature detection to ODWLogger.mm to conditionally compile Privacy Guard and Sanitizer integration. When modules aren't available (e.g., submodule not cloned), calls log a warning instead of failing to compile.
  • Updated Package.swift to add Privacy Guard and Sanitizer compilation conditions.
  • Updated Swift wrapper to use #if canImport() for optional module imports.

Updated samples

  • Added shared MSTelemetrySample.cmake for ease of updating all sample CMakeLists.txt files now include this shared module that handles both vcpkg (find_package) and legacy (vendored) dependency resolution.
  • Removed hardcoded install paths from all sample CMakeLists.txt files, since it was causing local builds to fail

Test infrastructure (tests/vcpkg/)

  • Added standalone vcpkg consumer test (main.cpp, CMakeLists.txt, vcpkg.json) using core APIs (LogManager, EventProperties, PII annotations, event priority) and verifies find_package(MSTelemetry CONFIG) works correctly.
  • Added platform-specific test scripts:
    - test-vcpkg-windows.ps1 — auto-detects host architecture (x64/ARM64), finds VS via vswhere, builds with x64-windows-static or arm64-windows-static, runs 8 integration tests.
    - test-vcpkg-linux.sh — detects host triplet, builds and runs tests.
    - test-vcpkg-macos.sh — builds with host-native triplet, runs tests.
    - test-vcpkg-ios.sh — builds for both arm64-ios (device) and simulator (arm64-ios-simulator), verifies Mach-O binary architecture.
    - test-vcpkg-android.sh — cross-compiles for configurable ABI (arm64-v8a, armeabi-v7a, x86_64, x86), verifies ELF binary production.
  • Added README.md with usage instructions for all platforms.

Bug fixes:
- Fix restartRecoversEventsFromStorage iOS functest failure
- FHttpRequestApple::Cancel() called [session getTasksWithCompletionHandler:] to cancel every task on the shared static NSURLSession.The completion handler callback fires asynchronously on an internal dispatch queue, so when one SDK session tears down and a new session starts sending requests, the stale callback arrives and cancels the new session's in-flight tasks. This caused receivedRequests.size() == 0 in the second block of the restart test. The fix removes the blanket cancel and keeps only [m_dataTask cancel], which cancels the specific task.

-  Fixed data races, memory leaks in HTTP pipeline and worker thread for Apple
    - Added m_httpCallbacksMtx around .empty() check in HttpClientManager: cancelAllRequests() which reads m_httpCallbacks.empty(), since onHttpResponse() removes entries under that mutex on another thread.
   - HttpResponseDecoder: The Abort outcome path set ctx->httpResponse = nullptr without delete, leaking the SimpleHttpResponse allocated by the HTTP client. Added the missing delete like RetryNetwork already had
   - WorkerThread: Join() logged warnings when queues were non-empty on shutdown but never freed the remaining Task* objects. Replaced the warnings with delete + clear().
   - TransmissionPolicyManager: uploadAsync() wrote m_runningLatency and m_scheduledUploadTime outside m_scheduledUploadMutex, while scheduleUpload() reads them under that mutex. Moved the writes inside the lock so the reader sees a consistent state. (Still some issues, since some other code touches them, but unlikely to be major issue)

Pipelines

  • Added vcpkg tests to pipelines
  • Updated macos-13 (removed in Dec. 2025) to macos-14
  • Cancelled previous pipeline runs if new commit is pushed
  • Extended some timeouts for tests to decrease flakiness, since builds running on simulators on slow or very active computers can be slow

Other changes

  • Added .gitattributes to marks tools/ports/, tests/vcpkg/, and docs/building-with-vcpkg.md with export-ignore so they are excluded from GitHub tarballs and so vcpkg files don't affect SHA needed for vcpkg, since vcpkg doesn't need the port files themselves when downloading the GitHub source tarball
  • Updated .gitignore to ignores vcpkg test build artifacts.
  • Fixed Logger.cpp destructor — removed LOG_TRACE call from ~Logger() to prevent static-destruction-order crash on iOS simulator (where debugLogMutex may be destroyed before the Logger instance). If this is an issue, I can revert it, as I am not sure if it would be an actual issue in a real iOS deployment scenario.
  • Removed stale header references from .vcxproj and .vcxproj.filters files (referenced files that no longer exist). If I was in doubt, I kept it in.
  • Updated docs (docs/building-with-vcpkg.md) to cover all platforms, overlay port usage, feature flags, and troubleshooting.

After merging

  • Need a separate commit to update the SHA and point REF at a release tag (I assume will be 3.10.42.1)
  • Then will officially submit port to vcpkg registry
    • May be possible to set it so that future releases will automatically result in updated vcpkg ports

For zlib vcpkg lacking SIMD patches:

  • zlib-ng does have the SIMD (and many other patches!), but setting ZLIB_COMPAT=on to use that would without changing any existing zlib calls would force downstream vcpkg users to also adopt zlib-ng. Downstream users can override zlib for each triplet themselves with something like:
set(ZLIB_COMPAT ON)

for the triplet .cmake file

Then, in vcpkg.json for the downstream consuming project, a user can use:

   {
     "dependencies": [
       "onnxruntime",
       "mstelemetry"
     ],
     "overrides": [
       { "name": "zlib", "version": "zlib-ng@2.3.3" }
     ]
   }

Another option for future consideration would be to use zlib-ng directly with ZLIB_COMPAT OFF (giving us a different namespace and preventing any package conflicts, duplicate symbols, and any issues that can come from that). This would also require updating the codebase to use the updated header files, which means that to prevent breaking the standard CMake build, we would have to also updated the vendored version to use zlib-ng (which would make updating the version easier but would require extensive testing).

bmehta001 and others added 30 commits March 4, 2026 03:04
Refactor the CMake build system to support vcpkg-provided dependencies
via find_package() when MATSDK_USE_VCPKG_DEPS is enabled. This is
auto-detected from the vcpkg toolchain but can also be set explicitly.

Changes:
- CMakeLists.txt: Add vcpkg detection, guard iOS/macOS manual flags and
  compiler flags behind vcpkg check, add find_package for sqlite3/zlib/
  nlohmann-json/curl, initialize BUILD_IOS/BUILD_APPLE_HTTP for config
  template, guard bondlite/include path for builds without submodules
- lib/CMakeLists.txt: Refactor dependency linking to use vcpkg CMake
  targets, add install(TARGETS/EXPORT) for CMake package config, fix
  Apple framework linking for both macOS and iOS (add CFNetwork, Network,
  UIKit), make /usr/local/include and ZLIB_WINAPI conditional on vcpkg
- cmake/MSTelemetryConfig.cmake.in: New config template for downstream
  find_package(MSTelemetry CONFIG) with platform-conditional deps
- tools/ports/mstelemetry/: Rewrite portfile.cmake to use pure CMake
  (vcpkg_cmake_configure/install/config_fixup), replace CONTROL with
  vcpkg.json manifest, remove legacy build scripts and patches, add usage
- tests/vcpkg/: Integration test suite with consumer project and
  per-platform test scripts (Windows/Linux/macOS/iOS)
- docs/building-with-vcpkg.md: Rewrite with modern instructions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Pin portfile REF + SHA512 so vcpkg manifest mode works without --head
- Rewrite test scripts to use CMake vcpkg toolchain integration instead
  of a separate vcpkg install step (avoids manifest mode conflicts)
- Windows script: use array splatting for cmake args, support -VcpkgRoot
  parameter to handle vcvarsall.bat overriding VCPKG_ROOT
- All platforms: check for vcpkg.cmake toolchain instead of vcpkg binary
- Update README with corrected 3-step process and vcvarsall troubleshooting

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Pin portfile REF to latest commit (55d7124) with correct SHA512
- Change curl dependency platform from 'linux' to '!windows & !osx & !ios'
  to cover any non-Windows/non-Apple platform (e.g., FreeBSD)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix C++ stdlib headers not found on macOS CLT: detect when headers
  exist only in the SDK sysroot and add -isystem path automatically
- Add missing -framework SystemConfiguration for ODWReachability
- Add MSTEL_LOCAL_SOURCE env override in portfile for local dev testing
- Add standalone consumer test project (tests/vcpkg-standalone/)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add export-ignore for tools/ports/ and tests/vcpkg/ in .gitattributes.
GitHub tarballs (used by vcpkg_from_github) will no longer include
these directories, breaking the chicken-and-egg cycle where portfile
SHA512 changes invalidate the tarball hash.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Now that .gitattributes excludes tools/ports/ from archives,
future portfile changes won't alter the tarball hash.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tion

The config template used @BUILD_IOS@ and @BUILD_APPLE_HTTP@ with mixed
YES/NO/ON/OFF values from the CMake build system. This is fragile because
CMake's boolean semantics can vary. Normalize both to canonical ON/OFF
before configure_package_config_file() and use explicit string comparison
in the template for robustness.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
FATAL_ERROR if MATSDK_BUILD_VERSION is not set or empty, preventing
write_basic_package_version_file from silently producing a broken
version file.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Apple frameworks: extract 5 common frameworks, differ only UIKit vs IOKit
- Merge tests/vcpkg-standalone/ into tests/vcpkg/ (7 tests now)
- Use ON/OFF for BUILD_IOS/BUILD_APPLE_HTTP from init, remove normalization
- Simplify MATSDK_BUILD_VERSION guard
- Config template uses native boolean eval (safe with guaranteed ON/OFF)
- Net -217 lines

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was temporary scaffolding for local testing without pushing code.
vcpkg provides built-in mechanisms (--head, --editable, overlay ports)
for local development. Official ports should only use vcpkg_from_github.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
REPO and HEAD_REF now reference the canonical upstream. REF and SHA512
still need updating after this branch is merged to main.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Link Android NDK log library when CMAKE_SYSTEM_NAME=Android
- Share Linux thread/dl linking with Android
- Update config template to find Threads on Android
- Android uses curl (CPP11 PAL) same as Linux, already covered by
  vcpkg.json platform filter

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Supports arm64-v8a (default), armeabi-v7a, x86_64, x86 ABIs.
Uses VCPKG_CHAINLOAD_TOOLCHAIN_FILE to chain the NDK toolchain
with vcpkg's toolchain. Cross-compiled binary verified by existence
and can be deployed to device via adb for runtime testing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…1, document duplication

- Restore robust MATSDK_BUILD_VERSION guard (check both undefined and empty)
- Update portfile REF to tagged release v3.10.42.1 (SHA512 placeholder)
- Update vcpkg.json version-semver to 3.10.42.1
- Add comments noting intentional C++ stdlib header workaround duplication

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- point the temporary pre-merge port at a specific tested commit on the branch fork
- add BUILD_VERSION so the generated CMake package version matches the port version
- use version-string for the 4-part SDK version in vcpkg.json

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- remove --head from the documented install commands
- note that --head is only for explicit unreleased branch/tag testing

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Instead of duplicating the entire include_directories() call in both
branches of the vcpkg conditional, keep a single include_directories()
with all internal paths and gate only /usr/local/include behind
if(NOT MATSDK_USE_VCPKG_DEPS).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add #ifndef guards in config-default.h to prevent HAVE_MAT_AI,
  HAVE_MAT_UTC, HAVE_MAT_SIGNALS, HAVE_MAT_SANITIZER redefinition
  warnings when both __has_include and CMake add_definitions set them.
  This was causing build failures with /WX (warnings-as-errors).
- Add clarifying comments to lib/CMakeLists.txt distinguishing legacy
  include_directories() from modern target_include_directories().
- Add detailed comments to MSTelemetryConfig.cmake.in explaining the
  CURL dependency logic and why @var@ substitution is used.
- Remove CMAKE_REQUIRED_LIBRARIES from legacy link lines (it is a
  CMake internal for check_* macros, not a link dependency).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add section for installing from the official vcpkg registry
- Keep overlay port section for development/pre-release use
- Add vcpkg manifest mode example (vcpkg.json snippet)
- Add Android platform instructions with NDK requirement
- Add Platforms column to dependency table
- Improve feature descriptions
- Update links to current Microsoft Learn URLs

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Use LANGUAGES keyword in project() for explicitness
- Add CMAKE_CXX_STANDARD_REQUIRED and CMAKE_CXX_EXTENSIONS
- Use get_filename_component for compiler dir (more robust)
- Use message(STATUS ...) instead of message("-- ...")
- Guard CRT matching with MSVC check
- Scope macOS header workaround to target via target_compile_options

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- message(STATUS ...) instead of message("-- ...") everywhere
  (root, lib, azmon, ParseOsRelease, MakeDeb/Rpm/Tgz, tests)
- project(MSTelemetry LANGUAGES C CXX) with CMAKE_CXX_STANDARD_REQUIRED
  and CMAKE_CXX_EXTENSIONS OFF
- Quote all EXISTS paths with variable expansions (CMP0054)
  (lib/CMakeLists.txt, tests/functests, tests/unittests)
- CMAKE_SYSTEM_NAME STREQUAL "Android" instead of bare ANDROID
- get_filename_component() for macOS CLT header workaround
- Remove redundant if(NOT CURL_FOUND) after find_package(CURL REQUIRED)
- Fix message(FATAL_ERROR, ...) to message(FATAL_ERROR ...) in swift wrapper
- find_package(Threads) extended to cover Android
- Portfile REPO restored to microsoft/cpp_client_telemetry
- Static CRT linkage guard scoped to MSVC in test consumer

Verified: legacy CMake build (mat target) passes, vcpkg 7/7 tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ortfile

- Restore accidentally deleted if-block bodies in tests/functests/CMakeLists.txt
- Remove outdated FIXME comment in tools/MakeTgz.cmake (arch already present)
- Use vcpkg \ variable instead of hardcoded BUILD_VERSION
- Switch portfile REPO to bmehta001 fork for branch validation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
REPO: bmehta001/cpp_client_telemetry
REF: ee6e6a0 (bhamehta/vcpkg-port-rearchitect branch)
HEAD_REF: bhamehta/vcpkg-port-rearchitect

This lets vcpkg tests build from the actual branch source code.
Revert to microsoft/cpp_client_telemetry with a release tag before
submitting the port upstream.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes z_size_t typedef missing from zconf.h.cmakein (CMake template
was out of sync with the actual zconf.h in 1.2.11). Also addresses
known CVEs in the old version.

The Chromium SIMD patches (crc_folding.c, fill_window_sse.c, x86.c/h)
and simd_stub.c are retained as-is — the SDK compiles simd_stub.c
which stubs out all SIMD calls.

Verified: Windows legacy CMake build (mat target) passes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Condense verbose tutorial-style comments to terse developer notes
- Remove duplicate PUBLIC keyword in legacy shared-lib link line
- Shorten macOS CLT workaround comments in root and test CMakeLists

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Match the pattern used by macOS, iOS, Android, and Windows scripts.
Detects aarch64 for arm64-linux, defaults to x64-linux.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Android uses HttpClient_Android, not curl. Exclude Android from curl
  dependency in vcpkg.json platform expression, config template, and docs.
- Simplify config template: capture MATSDK_NEEDS_CURL boolean at build
  time instead of re-deriving from three separate variables.
- Use 'version' instead of 'version-string' in vcpkg.json per vcpkg
  maintainer guidelines (dot-separated numeric versions).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove unused INSTALL_BIN/LIB/INC_DIR variables (GNUInstallDirs used)
- Remove CMAKE_SYSTEM_INFO_FILE status message (always empty)
- Collapse empty Clang/Intel/MSVC compiler branches to single GCC check
- Remove commented-out bondlite tests block
- Remove dead INSTALL_LIB_DIR assignments in packaging section

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bmehta001 and others added 16 commits March 16, 2026 10:24
Runs vcpkg consumer tests on all supported platforms:
- Windows (x64-windows-static)
- Linux (x64-linux)
- macOS (native arch)
- iOS (arm64-ios cross-compile)
- Android (arm64-v8a cross-compile)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
macos-13 runners were removed from GitHub Actions on Dec 4, 2025.
Also update actions/checkout to v4 and deployment target to 14.0.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace 'iPad Pro (11-inch) (4th generation)' with 'iPad Air 11-inch (M2)'
for macos-14 to avoid ambiguous xcodebuild destination matching. Use
'iPad Pro 11-inch (M4)' for macos-15.

Matrix:
  macos-14: iPhone 15, iPad Air 11-inch (M2)
  macos-15: iPhone 16, iPad Pro 11-inch (M4)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes ambiguous destination matching on macos-14 where xcodebuild
picks the 'Any iOS Device' placeholder over the simulator when
multiple OS versions exist. Adding OS=latest forces selection of
the newest concrete simulator runtime.

Reverts simulator name changes — the original names work correctly
with the explicit OS specifier.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
OS=latest is unreliable in newer Xcode versions. Instead, query
xcrun simctl for the newest available OS version matching the
requested simulator name. This eliminates ambiguous destination
matching that caused xcodebuild to pick 'Any iOS Device'.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the Python-based OS version detection with a simple grep
for the simulator UUID. Using -destination id=<UUID> is the most
reliable way to target a specific simulator, per xcodebuild docs
and StackOverflow consensus. No Python dependency needed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The test HTTP server binds with AF_INET (IPv4 only), but NSURLSession
on iOS 18 / macOS 15 resolves 'localhost' to ::1 (IPv6) first. The IPv6
connection is refused, and the IPv4 fallback combined with keepalive=false
causes intermittent -1005 'network connection was lost' errors when
NSURLSession tries to reuse a server-closed connection. Using 127.0.0.1
directly bypasses the IPv6 attempt entirely.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The default NSURLSession configuration caches and reuses TCP connections.
When the test HTTP server closes connections (keepalive=false), NSURLSession
can attempt to send on a stale connection, causing -1005 'network connection
was lost' errors. Ephemeral configuration disables connection caching,
ensuring each request gets a fresh connection. This also benefits production
use since the SDK's upload pattern (infrequent batched POSTs) does not
benefit from persistent connections.

Verified locally: 39/39 iOS func tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- PalTests.SystemTime: Widen upper bound from 550ms to 1000ms (sleep 369ms)
- PalTests.MonotonicTime: Widen upper bound from 950ms to 1500ms (sleep 789ms)
  Both still verify clocks work correctly; the wider bounds accommodate
  loaded CI runners hosting iOS simulators.
- APITest.LogManager_Initialize_DebugEventListener: Poll for up to 5s for
  the storage-full callback to arrive, since it fires asynchronously on
  the worker thread after Flush() returns.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…down

The NSURLSession was a file-level static shared across all SDK instances.
On teardown, CancelAllRequests returned before NSURLSession completion
handlers had finished executing on Apple's dispatch queue. The
HttpClient_Apple object (and its mutex) was then destroyed, and a stale
completion handler would crash trying to lock the destroyed mutex:
  'mutex lock failed: Invalid argument'

Fix: each HttpClient_Apple now owns its own ephemeral NSURLSession. The
destructor calls [invalidateAndCancel], which cancels all outstanding
tasks and blocks until every in-flight completion handler has finished,
guaranteeing no callbacks fire after the object is destroyed.

The header stores the session as void* since .hpp cannot reference Obj-C
types; the .mm file casts via __bridge_retained/__bridge_transfer for
proper ARC memory management.

Verified locally: 39/39 iOS tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When a new commit is pushed to the same branch, any in-progress workflow
run for that branch is automatically cancelled, avoiding wasted CI time.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bmehta001 bmehta001 force-pushed the bhamehta/vcpkg-port-rearchitect branch from 0ad2247 to 79f1782 Compare March 17, 2026 06:49
bmehta001 and others added 11 commits March 17, 2026 01:55
Move [invalidateAndCancel] from the destructor into CancelAllRequests(),
which runs while HttpClientManager and its callbacks are still alive
(during TelemetrySystem::onStop). This eliminates the theoretical window
where invalidateAndCancel could fire a stale completion handler after the
HttpClientManager is destroyed. The session is recreated lazily via
ensureSession()/GetOrCreateSession() if the SDK is reinitialized.

The destructor retains invalidateAndCancel as a safety net for cases
where CancelAllRequests was not called before destruction.

Verified locally: 39/39 iOS tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
v1 and v2 are deprecated; v3 will be soon. Update all active
workflow files to v4 for Node.js 20 support and latest fixes.
Disabled (.off) workflows are left unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- actions/checkout: v1/v2/v3 -> v4 (in .off files)
- actions/setup-java: v3 -> v5
- android-actions/setup-android: v2 -> v3
- microsoft/setup-msbuild: v1.1 -> v2
- actions/upload-artifact: v2 -> v4 (in .off file)

All updates move to Node.js 20+ runtimes, addressing GitHub's
deprecation of Node 16-based actions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
setup-android@v3 installs cmdline-tools at 16.0 (not 7.0) and
adds sdkmanager to PATH automatically. Remove the hardcoded
working-directory and call sdkmanager directly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace hardcoded C:\Users\runneradmin with $Env:USERPROFILE in
  build-android.yml and codeql-analysis.yml
- Update vs-version from [16,) (VS 2019+) to [17,) (VS 2022+) in
  test-win-latest.yml to match the windows-2022 runner

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…raint

- Change HEAD_REF from commit SHA to 'main' (vcpkg convention)
- Add comment noting BUILD_APPLE_HTTP must stay ON for macOS/iOS
  since curl is excluded from vcpkg.json on those platforms

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The queue cleanup must not run after detach() — the detached thread
still needs the shutdown item to exit its event loop. Cleaning up
after detach deletes the shutdown signal, causing the thread to hang
and then access destroyed members (use-after-free).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
m_runningLatency is read without locks at lines 219 and 376, while
written from other threads. Making it std::atomic eliminates the
undefined behavior without requiring additional locks (which could
deadlock through upload callbacks). The existing locks remain for
other shared state they protect.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sary)

Now that m_runningLatency is std::atomic, the write doesn't need
to be inside the lock scope. Restore the original position from
main to minimize the diff.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move m_runningLatency and m_scheduledUploadTime back to before the
LOCKGUARD scope, matching the original code on main. The .cpp now
has zero diff from main — all threading fixes are in the .hpp
(std::atomic declaration).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bmehta001 bmehta001 force-pushed the bhamehta/vcpkg-port-rearchitect branch from bad9bcf to ae941f9 Compare March 18, 2026 07:13
MSVC rejects passing std::atomic<T> to variadic functions (deleted
copy constructor). Use explicit .load() to pass the underlying
EventLatency value.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant