Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public typealias CrossPlatformApplication = NSApplication

@objcMembers public class TestNSNotificationCenterWrapper: NSObject {
private enum Observer {
case observerWithObject(WeakReference<NSObject>, Selector, NSNotification.Name?, Any?)
case observerWithObject(WeakReference<AnyObject>, Selector, NSNotification.Name?, Any?)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Observer removal fails for non-NSObject observers in test utility

The Observer.observerWithObject case was changed to use WeakReference<AnyObject> to support non-NSObject observers, but the removeObserver method's comparison logic at line 85 still uses observer as? NSObject. For non-NSObject observers, this cast returns nil, causing the identity comparison weakObserver.value === observer as? NSObject to fail, so observers won't be properly removed. The comparison should use observer as AnyObject instead.

Fix in Cursor Fix in Web

case observerForKeyPath(WeakReference<NSObject>, String, NSKeyValueObservingOptions, UnsafeMutableRawPointer?)
case observerWithBlock(WeakReference<NSObject>, NSNotification.Name?, (Notification) -> Void)
}
Expand All @@ -22,7 +22,7 @@ public typealias CrossPlatformApplication = NSApplication
private var observers: [Observer] = []

public var addObserverWithObjectInvocations = Invocations<(
observer: WeakReference<NSObject>,
observer: WeakReference<AnyObject>,
selector: Selector,
name: NSNotification.Name?,
object: Any?
Expand All @@ -34,8 +34,8 @@ public typealias CrossPlatformApplication = NSApplication
object anObject: Any? = nil
) {
if ignoreAddObserver == false {
addObserverWithObjectInvocations.record((WeakReference(value: observer as! NSObject), aSelector, aName, anObject))
observers.append(.observerWithObject(WeakReference(value: observer as! NSObject), aSelector, aName, anObject))
addObserverWithObjectInvocations.record((WeakReference(value: observer as AnyObject), aSelector, aName, anObject))
observers.append(.observerWithObject(WeakReference(value: observer as AnyObject), aSelector, aName, anObject))
}
}

Expand Down
12 changes: 5 additions & 7 deletions Sources/Swift/Integrations/Session/SessionTracker.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ typealias Application = NSApplication

/// Tracks sessions for release health. For more info see:
/// https://docs.sentry.io/workflow/releases/health/#session
@_spi(Private) @objc(SentrySessionTracker) public final class SessionTracker: NSObject {
final class SessionTracker {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Removing NSObject breaks NotificationCenter selector-based observation

The SessionTracker class no longer inherits from NSObject, but it still uses #selector with NotificationCenter.addObserver(_:selector:name:object:) for lifecycle notifications. Selector-based notification observation requires the observer to inherit from NSObject because the Objective-C runtime needs methods like methodSignatureForSelector: to dispatch the selector. Without NSObject inheritance, the notification callbacks (didBecomeActive, willResignActive, willTerminate) will fail at runtime, breaking session tracking functionality.

Fix in Cursor Fix in Web


// MARK: Private

Expand Down Expand Up @@ -39,9 +39,9 @@ typealias Application = NSApplication
self.notificationCenter.removeObserver(self, name: nil, object: nil)
}

// MARK: Public
// MARK: Internal

@objc public func start() {
func start() {
// We don't want to use WillEnterForeground because tvOS doesn't call it when it launches an app
// the first time. It only calls it when the app was open and the user navigates back to it.
// DidEnterBackground is called when the app launches a background task so we would need to
Expand Down Expand Up @@ -73,7 +73,7 @@ typealias Application = NSApplication
#endif
}

@objc public func stop() {
func stop() {
SentrySDKInternal.currentHub().endSession()

removeObservers()
Expand All @@ -83,7 +83,7 @@ typealias Application = NSApplication
wasStartSessionCalled = false
}

@objc public func removeObservers() {
func removeObservers() {
#if ((os(iOS) || os(tvOS) || (swift(>=5.9) && os(visionOS))) && !SENTRY_NO_UIKIT) || ((os(macOS) || targetEnvironment(macCatalyst)) && !SENTRY_NO_UIKIT)
// Remove the observers with the most specific detail possible, see
// https://developer.apple.com/documentation/foundation/nsnotificationcenter/1413994-removeobserver
Expand All @@ -94,8 +94,6 @@ typealias Application = NSApplication
#endif
}

// MARK: Internal

let options: Options
let dateProvider: SentryCurrentDateProvider
let notificationCenter: SentryNSNotificationCenterWrapper
Expand Down
2 changes: 1 addition & 1 deletion Sources/Swift/SentryDependencyContainer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ extension SentryFileManager: SentryFileManagerProtocol { }
return defaultApplicationProvider()
}

@objc(getSessionTrackerWithOptions:) public func getSessionTracker(with options: Options) -> SessionTracker {
func getSessionTracker(with options: Options) -> SessionTracker {
return SessionTracker(options: options, applicationProvider: defaultApplicationProvider, dateProvider: dateProvider, notificationCenter: notificationCenterWrapper)
}

Expand Down
Loading