diff --git a/CHANGELOG.md b/CHANGELOG.md index 64b07e681a3..073ba51fd09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,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) +- Resolve crash in caused by calling `SentryFramesTracker.removeListener(_:)` (#7155) ## 9.1.0 diff --git a/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift b/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift index 8883804d8e6..91a91cafec1 100644 --- a/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift +++ b/Sources/Swift/Core/Integrations/FramesTracking/SentryFramesTracker.swift @@ -18,12 +18,25 @@ public protocol SentryFramesTrackerListener: NSObjectProtocol { @_spi(Private) @objc public class SentryFramesTracker: NSObject { - - var isStarted: Bool = false - @objc public private(set) var isRunning: Bool = false + + private var isStarted: Bool = false // MARK: Private properties - private var lock = NSLock() + 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 @@ -68,11 +81,23 @@ public class SentryFramesTracker: NSObject { resetFrames() SentrySDKLog.debug("Initialized frame tracker") } + + deinit { + removeObservers() + // Need to invalidate so DisplayLink is releasing this object. Calling this is thread-safe. + displayLinkWrapper.invalidate() + } // MARK: - Public Methods @objc public func start() { + dispatchQueueWrapper.dispatchAsyncOnMainQueueIfNotMainThread { + self.startInternal() + } + } + + private func startInternal() { guard !isStarted else { return } isStarted = true @@ -96,12 +121,24 @@ public class SentryFramesTracker: NSObject { @objc public func stop() { + dispatchQueueWrapper.dispatchAsyncOnMainQueueIfNotMainThread { + self.stopInternal() + } + } + + private func stopInternal() { guard isStarted else { return } isStarted = false pause() + removeObservers() + + listeners.removeAllObjects() + } + + private func removeObservers() { notificationCenter.removeObserver( self, name: CrossPlatformApplication.didBecomeActiveNotification, @@ -113,10 +150,6 @@ public class SentryFramesTracker: NSObject { name: CrossPlatformApplication.willResignActiveNotification, object: nil ) - - lock.synchronized { - listeners.removeAllObjects() - } } @objc @@ -170,10 +203,6 @@ public class SentryFramesTracker: NSObject { self.listeners.remove(listener) } } - - deinit { - stop() - } #if os(iOS) @objc public func resetProfilingTimestamps() { @@ -222,7 +251,7 @@ public class SentryFramesTracker: NSObject { private func willResignActive() { pause() } - + private func unpause() { guard !isRunning else { return } @@ -243,7 +272,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 @@ -332,7 +361,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() diff --git a/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift b/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift index 521c17398a3..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 { @@ -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, 2) + + sut.stop() + + XCTAssertEqual(dispatchQueueWrapper.blockOnMainInvocations.count, 3) + } + func testRemoveListener() { let sut = fixture.sut let listener = FrameTrackerListener()