Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NSInvalidArgumentException unrecognized selector crash due to OverlayScrollViewDelegateProxy deallocation issue #100

Open
bshuttkg opened this issue Jan 20, 2025 · 3 comments

Comments

@bshuttkg
Copy link

Describe the bug

Memory deallocation issue in the OverlayScrollViewDelegateProxy.

Environnement

  • Device: iPhone 16 Pro Simulator
  • OS: iOS 18.2
  • OverlayContainer Version: aa7bd20

Description

As you know, the proxy sets itself as the UIScrollViewDelegate:

scrollView.delegate = self

For our use case, the scrollView is a UITableView; so the delegate is a UITableViewDelegate.
As a result, it will be sent messages like tableView:viewForHeaderInSection:.

The framework does consider this, as we see in the following code to forward on other messages:

override func responds(to aSelector: Selector!) -> Bool {
    let originalDelegateRespondsToSelector = originalDelegate?.responds(to: aSelector) ?? false
    return super.responds(to: aSelector) || originalDelegateRespondsToSelector
}

override func forwardingTarget(for aSelector: Selector!) -> Any? {
    if originalDelegate?.responds(to: aSelector) == true {
        return originalDelegate
    } else {
        return super.forwardingTarget(for: aSelector)
    }
}

where:

private weak var originalDelegate: UIScrollViewDelegate?

But there is a bug here, a few:

  1. In ARC in Swift: Basics and Beyond, we see how Swift is non-lexical. Above, in forwardingTarget(for:), originalDelegate may respond to a selector and be deallocated when it is returned (because it is weak)
  2. originalDelegate may cause responds(to:) to return true, but have been deallocated in forwardingTarget(for:)

The second is why we are seeing crashes in our app.
Just before the crash, forwardingTarget(for:) is called with selector tableView:viewForHeaderInSection: and the originalDelegate is nil.

Returning nil for this scenario in forwardingTarget(for:) does not stop the invocation.

So we are experiencing the following crash in our app:

'NSInvalidArgumentException', reason: '-[OverlayContainer.OverlayScrollViewDelegateProxy tableView:viewForHeaderInSection:]: unrecognized selector sent to instance'

It looks to me like you might have done the same as this SO answer.
Currently, I don't think it's a correct implementation of a proxy in Swift for weak references.

@bshuttkg bshuttkg changed the title NSInvalidArgumentException Unrecognized Selector Crash Due To OverlayScrollViewDelegateProxy Deallocation Issue NSInvalidArgumentException unrecognized selector crash due to OverlayScrollViewDelegateProxy deallocation issue Jan 20, 2025
@gaetanzanella
Copy link
Contributor

Hi @bshuttkg, thank you for the feedback!

Could you try to make a PR for it? 🙏
There is a test and it sounds like you understand the problem :)

@bshuttkg
Copy link
Author

Hey @gaetanzanella, thank you for getting back to me 🙏 .

Currently, I'm not sure that there is a Swift solution for the selector proxy pattern we are attempting here.
This might be an Objective-C thing, but I would like to be wrong about that.

I've tried returning nil in forwardingTarget(for:) when the originalDelegate is deallocated, but the invocation seems to be invoked anyway.

If Objective-C was an option (probably not) then:

  • This blog discusses how this might be achieved using NSObject
  • WeakForwarder is an implementation using NSProxy

@gaetanzanella
Copy link
Contributor

Did you succeed to create a test to reproduce the issue? in OverlayScrollViewDelegateTest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants