Rearchitect vcpkg port for official submission#1
Open
Conversation
8c1d2b5 to
0999317
Compare
7a5f685 to
0d9bb76
Compare
…microsoft#1417) * Fix JNI stale local ref in OfflineStorage_Room record iteration loops GetAndReserveRecords, GetRecords, and ReleaseRecords each use a pushLocalFrame/popLocalFrame scope per record. The jclass obtained from GetObjectClass on the first iteration was stored as a plain local reference; after popLocalFrame returns, that reference is freed by ART. On subsequent iterations the C++ pointer is non-null, so the if(!record_class) guard is skipped, but the jclass* now points into a freed local frame. ART's JNI reference validity checker detects this and calls JniAbort, producing SIGABRT. Fix: promote the jclass to a global reference via NewGlobalRef immediately after GetObjectClass so it survives across popLocalFrame calls, and delete it with DeleteGlobalRef after the loop. Affected methods: - GetAndReserveRecords: record_class - ReleaseRecords: bt_class - GetRecords: record_class The bug was introduced in commit 873b562 (Jan 2021) when per-record pushLocalFrame/popLocalFrame scoping was added. See GitHub issue microsoft#1227. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(OfflineStorage_Room): use RAII guard for JNI global refs The initial fix (previous commit) correctly promoted jclass local refs to global refs to survive pushLocalFrame/popLocalFrame, but placed DeleteGlobalRef only after the loop in normal flow. Three paths leaked: 1. ThrowLogic throws std::logic_error — NOT caught by catch(runtime_error) 2. ThrowRuntime throws std::runtime_error — catch block never called Delete 3. consumer() throwing in GetAndReserveRecords also bypassed the delete Fix: replace post-loop DeleteGlobalRef with a RAII guard struct whose destructor calls DeleteGlobalRef unconditionally. Applied to all three methods: GetAndReserveRecords, ReleaseRecords, and GetRecords. Also: - Initialize all jfieldID variables to nullptr (prevents use of indeterminate values if ThrowLogic fires mid field-ID setup) - Check NewGlobalRef return value; throw std::runtime_error on null (caught by existing catch block) Regression test improvements: - GetRecords section: add order-independent set-based blob[0] check to verify field IDs cached on iteration 0 are valid on iterations 1+ - ReleaseRecords section: cycle GetAndReserveRecords + ReleaseRecords (incrementRetryCount=true) GetMaximumRetryCount()+1 times so the Room impl actually drops records and returns a non-empty byTenant array, exercising the bt_class loop. Without this, bt_class path was never entered. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: consolidate GlobalRefGuard; fix build & test portability OfflineStorage_Room.cpp: - Move GlobalRefGuard struct into the anonymous namespace at the top of the file, replacing three identical inline local struct definitions. Eliminates duplication and makes a single authoritative definition. - Change guard member from jclass& ref to jclass* ref_ptr for cleaner pointer semantics (more idiomatic RAII ownership pattern). Tests: - GetAndReserveRecords blob check: switch from index-based to set-based comparison since Memory storage returns records in LIFO order, not insertion order. - GetRecords check: guard with if(!Memory) because Memory::GetRecords delegates to GetAndReserveRecords internally and returns nothing when records are already reserved. Build fixes (macOS Apple Silicon): - lib/CMakeLists.txt: use find_library to set IMPORTED_LOCATION on the sqlite3 IMPORTED GLOBAL target; without this cmake fails to generate on Apple Silicon where homebrew lives under /opt/homebrew. - tests/unittests/CMakeLists.txt: add /opt/homebrew/opt/sqlite/lib path to the sqlite3 library lookup chain. - lib/modules: update submodule to fix -Werror,-Wreorder-ctor build failure in Sanitizer.cpp (initializer list order mismatch). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Hansel Ip <hanselip@microsoft.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…oad (microsoft#1422) * disable built-in SDK stats events by default to reduce OneCollector load * Fix BasicFuncTests
0d9bb76 to
df8b674
Compare
* Modernize CI workflows and fix iOS simulator selection - Update Actions to latest versions (checkout@v4, setup-java@v5, etc.) - Add concurrency groups scoped to PR events only (push builds on main always run to completion) - Add 30-minute timeout to iOS CI to prevent deadlock hangs - Fix iOS simulator UUID resolution for unambiguous destination - Fix Android SDK path detection in CodeQL workflow - Document python3 requirement for iOS tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix compiler flags and scope -Wno-reorder - Split GCC/Clang warning flags (Clang-only flags were breaking GCC) - Add -fno-finite-math-only to restore IEEE 754 compliance needed by sqlite3 when -ffast-math is enabled - Remove global -Wno-reorder, scope to Sanitizer.cpp only (submodule declares members in wrong order) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Improve iOS test script and documentation - Resolve simulator UUID for unambiguous xcodebuild destination - Add python3 dependency documentation - Update iOS getting-started docs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix flaky functional tests for iOS simulator CI - Use unique phase2 DB paths instead of sleep to avoid SQLite vnode collisions from deferred sqlite3_close_v2 cleanup - Clean SQLite journal files (-wal, -shm, -journal) in CleanStorage - Fix cross-test config contamination (reset storage settings) - Use sleep(10) instead of yield() in waitForEvents to reduce CPU contention on single-core simulator runners - Increase timeouts for iOS simulator environment - Fix multi-batch upload flakiness with SetTransmitProfile(RealTime) - Use 127.0.0.1 instead of localhost for local test server Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6f874e4 to
5405100
Compare
* Remove private in6 header includes Remove the dead netinet6/in6.h imports from ODWReachability and its Objective-C unit test so Apple builds no longer fail on the private-header error from main. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove dead reachability imports Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Restore explicit reachability socket headers Add back the canonical sys/socket.h and netinet/in.h includes in ODWReachability.m and ODWReachabilityTests.mm while keeping the other dead imports removed. This avoids relying on transitive Apple headers in the reachability PR review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Modernize CMakeLists.txt: flatten nesting, deduplicate, use consistent quoting and variable patterns - Remove stale header references from vcxproj and vcxitems files - Simplify test CMakeLists.txt files - Fix CMake conventions in packaging scripts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ffc37ed to
ed30a8a
Compare
Keep the before.targets toolset selection deterministic on newer Visual Studio hosts, but only set the MIP props fallback when a consumer has not already chosen a toolset. While addressing the MSBuild review comments, also point the optional module test conditions and source paths at the real lib/modules locations and use CPACK_PACKAGE_FILE_NAME for the RPM status message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6d9b5c4 to
c857e4b
Compare
Use a real ZLIB package in the non-vcpkg Linux/macOS path instead of a bare imported z target with no location. This restores the regular debug test path on Ubuntu/macOS while keeping the vendored-vs-vcpkg dependency split intact. Files changed: - lib/CMakeLists.txt - tests/functests/CMakeLists.txt - tests/unittests/CMakeLists.txt Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes:
vcpkg
CMake (had to modify to fix some behavior in legacy builds and for vcpkg)
(-lsqlite3 -lz).
Build flags
Obj-C/Swift wrapper resilience
Updated samples
Test infrastructure (tests/vcpkg/)
- 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.
Other changes
After merging