From 46d119be1e8bba04b211759eea4a638c4e2f5a53 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Mon, 12 Jan 2026 13:39:27 +0100 Subject: [PATCH 1/9] fix: [SentryFramesTracker removeListener:] crash Mutate listners in `SentryFramesTracker` only on main queue. --- .../FramesTracking/SentryFramesTracker.swift | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift b/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift index ba4c22b0b5c..45a862d9f95 100644 --- a/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift +++ b/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift @@ -23,7 +23,6 @@ public class SentryFramesTracker: NSObject { @objc public private(set) var isRunning: Bool = false // MARK: Private properties - private var lock = NSLock() private var previousFrameTimestamp: CFTimeInterval = SentryFramesTracker.previousFrameInitialValue private var previousFrameSystemTimestamp: UInt64 = 0 private var currentFrameRate: UInt64 = 60 @@ -114,9 +113,7 @@ public class SentryFramesTracker: NSObject { object: nil ) - lock.synchronized { - listeners.removeAllObjects() - } + removeAllListeners() } @objc @@ -170,6 +167,12 @@ public class SentryFramesTracker: NSObject { self.listeners.remove(listener) } } + + func removeAllListeners() { + dispatchQueueWrapper.dispatchAsyncOnMainQueueIfNotMainThread { + self.listeners.removeAllObjects() + } + } deinit { stop() From 55e0b98ae1e95230572453109ab4fa6f2771f074 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Mon, 12 Jan 2026 13:48:08 +0100 Subject: [PATCH 2/9] test if removla is on main --- .../FramesTracking/SentryFramesTrackerTests.swift | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift b/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift index 521c17398a3..3629c05f052 100644 --- a/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift +++ b/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift @@ -775,6 +775,21 @@ class SentryFramesTrackerTests: XCTestCase { XCTAssertEqual(dispatchQueueWrapper.blockOnMainInvocations.count, 1) } + func testListenersAreRemovedInMainThread() { + let dispatchQueueWrapper = TestSentryDispatchQueueWrapper() + let sut = SentryFramesTracker(displayLinkWrapper: fixture.displayLinkWrapper, dateProvider: fixture.dateProvider, dispatchQueueWrapper: dispatchQueueWrapper, notificationCenter: fixture.notificationCenter, delayedFramesTracker: TestDelayedWrapper(keepDelayedFramesDuration: fixture.keepDelayedFramesDuration, dateProvider: fixture.dateProvider)) + let listener = FrameTrackerListener() + + sut.addListener(listener) + sut.start() + + XCTAssertEqual(dispatchQueueWrapper.blockOnMainInvocations.count, 1) + + sut.stop() + + XCTAssertEqual(dispatchQueueWrapper.blockOnMainInvocations.count, 2) + } + func testRemoveListener() { let sut = fixture.sut let listener = FrameTrackerListener() From 30c50f3166e717603b5461135726713391ada26a Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Mon, 12 Jan 2026 13:51:28 +0100 Subject: [PATCH 3/9] Add cl entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 24e7e40e95a..a52e24727bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ - Fix `raw_description` in `runtime` context on Mac Catalyst (#7082) - Deprecates `configureDarkTheme` for user feedback (#7114) - Fix incorrect variable assignment for 'sampled' key (#7120) +- [SentryFramesTracker removeListener:] crash ([#7155](https://github.com/getsentry/sentry-cocoa/pull/7155)) ## 9.1.0 From 0c6ba3dae042296e21e49e8789049aee3bd9120a Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Mon, 12 Jan 2026 16:30:23 +0100 Subject: [PATCH 4/9] update cl --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a52e24727bb..bfa36238a16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,8 @@ - Fix `raw_description` in `runtime` context on Mac Catalyst (#7082) - Deprecates `configureDarkTheme` for user feedback (#7114) - Fix incorrect variable assignment for 'sampled' key (#7120) -- [SentryFramesTracker removeListener:] crash ([#7155](https://github.com/getsentry/sentry-cocoa/pull/7155)) +- [SentryFramesTracker removeListener:] crash +- Resolve crash in caused by calling `SentryFramesTracker.removeListener(_:)` (#7155) ## 9.1.0 From 9f149dae65050b4225eba859c5ee833ed172273f Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Tue, 13 Jan 2026 10:51:38 +0100 Subject: [PATCH 5/9] fix changelog --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bfa36238a16..a79e530152b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,6 @@ - Fix `raw_description` in `runtime` context on Mac Catalyst (#7082) - Deprecates `configureDarkTheme` for user feedback (#7114) - Fix incorrect variable assignment for 'sampled' key (#7120) -- [SentryFramesTracker removeListener:] crash - Resolve crash in caused by calling `SentryFramesTracker.removeListener(_:)` (#7155) ## 9.1.0 From 6ed3052b9956fc108f42fb172d63357b48a1244a Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Tue, 13 Jan 2026 12:14:51 +0100 Subject: [PATCH 6/9] additional crash guards - Updated `SentryFramesTracker` to ensure listeners are managed on the main queue, preventing potential crashes. - Made `isStarted` property private for better encapsulation. - Enhanced deinitialization to safely stop the tracker and avoid async dispatch issues. --- .../FramesTracking/SentryFramesTracker.swift | 37 ++++++++++++------- .../SentryFramesTrackerTests.swift | 6 +-- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift b/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift index 45a862d9f95..5a9085718ca 100644 --- a/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift +++ b/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift @@ -18,8 +18,8 @@ public protocol SentryFramesTrackerListener: NSObjectProtocol { @_spi(Private) @objc public class SentryFramesTracker: NSObject { - - var isStarted: Bool = false + + private var isStarted: Bool = false @objc public private(set) var isRunning: Bool = false // MARK: Private properties @@ -67,11 +67,24 @@ public class SentryFramesTracker: NSObject { resetFrames() SentrySDKLog.debug("Initialized frame tracker") } + + deinit { + // Avoid async dispatch with self capture on deinit. + dispatchQueueWrapper.dispatchSyncOnMainQueue { + self.stopInternal() + } + } // MARK: - Public Methods @objc public func start() { + dispatchQueueWrapper.dispatchAsyncOnMainQueueIfNotMainThread { + self.startInternal() + } + } + + private func startInternal() { guard !isStarted else { return } isStarted = true @@ -95,6 +108,12 @@ public class SentryFramesTracker: NSObject { @objc public func stop() { + dispatchQueueWrapper.dispatchAsyncOnMainQueueIfNotMainThread { + self.stopInternal() + } + } + + private func stopInternal() { guard isStarted else { return } isStarted = false @@ -113,7 +132,7 @@ public class SentryFramesTracker: NSObject { object: nil ) - removeAllListeners() + listeners.removeAllObjects() } @objc @@ -168,16 +187,6 @@ public class SentryFramesTracker: NSObject { } } - func removeAllListeners() { - dispatchQueueWrapper.dispatchAsyncOnMainQueueIfNotMainThread { - self.listeners.removeAllObjects() - } - } - - deinit { - stop() - } - #if os(iOS) @objc public func resetProfilingTimestamps() { // The DisplayLink callback always runs on the main thread. We dispatch this to the main thread @@ -225,7 +234,7 @@ public class SentryFramesTracker: NSObject { private func willResignActive() { pause() } - + private func unpause() { guard !isRunning else { return } diff --git a/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift b/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift index 3629c05f052..62062b30c86 100644 --- a/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift +++ b/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift @@ -7,7 +7,7 @@ import XCTest // This test class also includes tests for delayed frames calculation which is quite complex. #if os(iOS) || os(tvOS) || targetEnvironment(macCatalyst) -class SentryFramesTrackerTests: XCTestCase { +final class SentryFramesTrackerTests: XCTestCase { private class Fixture { @@ -783,11 +783,11 @@ class SentryFramesTrackerTests: XCTestCase { sut.addListener(listener) sut.start() - XCTAssertEqual(dispatchQueueWrapper.blockOnMainInvocations.count, 1) + XCTAssertEqual(dispatchQueueWrapper.blockOnMainInvocations.count, 2) sut.stop() - XCTAssertEqual(dispatchQueueWrapper.blockOnMainInvocations.count, 2) + XCTAssertEqual(dispatchQueueWrapper.blockOnMainInvocations.count, 3) } func testRemoveListener() { From cd4ec9ac41399bbe125a8b96e9a0c4409d0d2f99 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Tue, 13 Jan 2026 16:43:16 +0100 Subject: [PATCH 7/9] only invalidate displayLinkWrapper in deinit --- .../Integrations/FramesTracking/SentryFramesTracker.swift | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift b/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift index 5a9085718ca..d467754c1c3 100644 --- a/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift +++ b/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift @@ -69,10 +69,8 @@ public class SentryFramesTracker: NSObject { } deinit { - // Avoid async dispatch with self capture on deinit. - dispatchQueueWrapper.dispatchSyncOnMainQueue { - self.stopInternal() - } + // Need to invalidate so DisplayLink is releasing this object. Calling this is thread-safe. + displayLinkWrapper.invalidate() } // MARK: - Public Methods From cbc4627e0697df776ba9657b114a43613209330f Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Mon, 19 Jan 2026 11:30:12 +0100 Subject: [PATCH 8/9] Remove observers in deinit --- .../FramesTracking/SentryFramesTracker.swift | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift b/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift index d467754c1c3..cdfaf3fdf6e 100644 --- a/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift +++ b/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift @@ -69,6 +69,7 @@ public class SentryFramesTracker: NSObject { } deinit { + removeObservers() // Need to invalidate so DisplayLink is releasing this object. Calling this is thread-safe. displayLinkWrapper.invalidate() } @@ -118,6 +119,12 @@ public class SentryFramesTracker: NSObject { pause() + removeObservers() + + listeners.removeAllObjects() + } + + private func removeObservers() { notificationCenter.removeObserver( self, name: CrossPlatformApplication.didBecomeActiveNotification, @@ -129,8 +136,6 @@ public class SentryFramesTracker: NSObject { name: CrossPlatformApplication.willResignActiveNotification, object: nil ) - - listeners.removeAllObjects() } @objc @@ -253,7 +258,7 @@ public class SentryFramesTracker: NSObject { displayLinkWrapper.invalidate() } -// swiftlint:disable file_length function_body_length +// swiftlint:disable function_body_length @objc private func displayLinkCallback() { let thisFrameTimestamp = displayLinkWrapper.timestamp @@ -342,7 +347,7 @@ public class SentryFramesTracker: NSObject { previousFrameSystemTimestamp = thisFrameSystemTimestamp reportNewFrame() } -// swiftlint:enable file_length function_body_length +// swiftlint:enable function_body_length private func reportNewFrame() { let newFrameDate = dateProvider.date() From fc620db8129c9b7062de84885254da4a1c9c94c4 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Tue, 20 Jan 2026 12:04:08 +0100 Subject: [PATCH 9/9] Introduced a private lock and backing variable to ensure thread-safe access to the isRunning property. --- .../FramesTracking/SentryFramesTracker.swift | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift b/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift index 0ff05f6f578..91a91cafec1 100644 --- a/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift +++ b/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift @@ -20,9 +20,23 @@ public protocol SentryFramesTrackerListener: NSObjectProtocol { public class SentryFramesTracker: NSObject { private var isStarted: Bool = false - @objc public private(set) var isRunning: Bool = false // MARK: Private properties + private var isRunningLock = NSLock() + private var _isRunning: Bool = false + + @objc public private(set) var isRunning: Bool { + get { + return isRunningLock.synchronized { + return _isRunning + } + } + set { + isRunningLock.synchronized { + _isRunning = newValue + } + } + } private var previousFrameTimestamp: CFTimeInterval = SentryFramesTracker.previousFrameInitialValue private var previousFrameSystemTimestamp: UInt64 = 0 private var currentFrameRate: UInt64 = 60