ref: Remove enableWatchdogTerminationsV2 flag#8321
Draft
philprime wants to merge 60 commits into
Draft
Conversation
Introduce a layered design for hang detection: - RunLoopDelayTracker (renamed from DefaultHangTracker): raw ~25ms runloop polling via CFRunLoopObserver + semaphore - HangTracker (new middle layer): debounces delays into hangs with per-observer thresholds, notifies once on threshold crossing and once on hang end - WatchdogTerminationTrackingIntegration: now uses HangTracker.addObserver(threshold:) instead of manually filtering by duration
Gives the middle layer a distinct name so git sees the original HangTracker.swift → RunLoopDelayTracker.swift as a clean rename instead of a full rewrite.
Backport of Synchronization.Mutex API (iOS 18+) using os_unfair_lock for older deployment targets.
Document SentryMutex as preferred locking primitive for new Swift code. Expose isSessionPaused as computed property for test access after State struct migration.
Move sentrycrashbic callback registration back inside the mutex to prevent a race between start() and stop(), matching the original behavior.
This reverts commit 3bf7671.
Register callbacks outside the lock since sentrycrashbic_registerAddedCallback synchronously replays loaded images, each re-entering the lock. Unregistering stays inside the lock as it does not replay.
…observer-threshold
Add doc comments to SentryRunLoopDelay and SentryAppHang structs. Document the conditional compilation protocol/typealias pattern and the CFRunLoop function type abstractions. Fix class doc referencing ongoing=true/false instead of .started/.ended. Tighten addObserver doc, fix typos, remove redundant inline comments that restate what the code already expresses.
Clarifies which tracker the token belongs to and fixes trailing whitespace.
- Use UnsafeMutablePointer for os_unfair_lock to guarantee a stable heap-allocated address - Make SentryMutex internal (not public API) - Drop @unchecked Sendable from Storage class - Update doc comment to reference OSAllocatedUnfairLock
The threshold check used > which meant a delay exactly equal to the configured threshold would not trigger a notification.
AppHangTracker: mid-hang observer add/remove, re-add during hang, double-remove, stop/restart lifecycle, duration assertions. RunLoopDelayTracker: partial observer removal, observer added during active hang, double-remove same token.
Replace NSRecursiveLock with SentryMutex<T> for both trackers. The lock is non-recursive but safe here because handlers are called from the background queue while add/remove are main-queue-only, so re-entrance cannot occur.
The threshold check should use strict greater-than (>) to match the watchdog termination tracking behavior. A delay exactly equal to the threshold should not trigger a notification.
Replace MockAppHangTracker (which duplicated threshold logic) with a MockRunLoopDelayTracker injected into the real SentryDefaultAppHangTracker. This ensures tests exercise the actual production comparison logic instead of a mock copy.
…observer-threshold
Replace standalone SentryRunLoopDelayTrackerDependencies protocol with a typealias composing DateProviderProvider & ApplicationProvider. Update all tests to use MockDependencies instead of the removed dateProvider: init parameter.
getKeyWindow() was reading UIWindow.isKeyWindow on the caller's thread after getWindows() returned. This is unsafe because isKeyWindow is a UIKit property that must be accessed on the main thread. Wrap the entire operation in dispatchSyncOnMainQueue via a new internal_getKeyWindow() extension method.
- Rename addOngoingHangObserver to addObserver - Rename parameter id to token for observer tokens - Rename hangNotifyThreshold to pollingInterval - Replace NSRecursiveLock with SentryMutex - Reorganize class layout with MARK sections - Convert docs from comments to doc comments - Add tests for multi-observer edge cases
- Rename addOngoingHangObserver to addObserver - Rename parameter id to token for observer tokens - Rename hangNotifyThreshold to pollingInterval - Replace NSRecursiveLock with SentryMutex - Reorganize class layout with MARK sections - Convert docs from comments to doc comments - Add tests for multi-observer edge cases
…cker-per-observer-threshold
…cker-per-observer-threshold
…r-refactoring-5 # Conflicts: # Sources/Swift/Integrations/AppHangTracking/SentryRunLoopDelayTracker.swift # Sources/Swift/Integrations/WatchdogTerminations/SentryWatchdogTerminationTrackingIntegration.swift # Tests/SentryTests/Integrations/AppHangTracking/SentryRunLoopDelayTrackerTests.swift # Tests/SentryTests/Integrations/WatchdogTerminations/SentryWatchdogTerminationTrackingIntegrationTests.swift
- Clarify comment about deferred deallocation outside lock - Use inverted XCTestExpectation for thread-safe removed-observer check - Update values before fulfilling expectation to avoid race condition
…cker-per-observer-threshold
Make SentryAppHangTracker the default for watchdog termination tracking, removing the experimental opt-in flag. Both the hang tracking integration and watchdog termination integration now use the same underlying hang tracker. Remove enableWatchdogTerminationsV2 from SentryExperimentalOptions, the ObjC header, and the ObjC wrapper. Update tests to use MockAppHangTracker directly. Regenerate public API surface.
8 tasks
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
1 participant
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.
Description
Remove the
enableWatchdogTerminationsV2experimental flag and makeSentryAppHangTrackerthe default for watchdog termination tracking.The watchdog termination integration now always uses the shared
SentryAppHangTrackerobserver pattern instead of maintaining a separateSentryANRTrackercode path behind an opt-in flag.Motivation and Context
With PR #8320 migrating
SentryHangTrackingIntegrationtoSentryAppHangTracker, both integrations should use the same underlying hang tracker. Keeping the V1 ANR tracker code path behind a flag adds maintenance burden and diverges the two integrations. Removing the flag simplifies the codebase and ensures consistent hang detection behavior across both integrations.How did you test it?
SentryWatchdogTerminationIntegrationTestspass (0 unexpected failures)SentryObjCExperimentalOptionsTestspass after removing the V2 flag testsmake build-iossucceedsmake generate-public-apirun to regenerate API surfaceChecklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.