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

Fix: objc_getAssociatedObject crashing on weak reference #161

Conversation

nuno-vieira
Copy link
Collaborator

@nuno-vieira nuno-vieira commented Dec 16, 2021

Description

It looks that the previous fix: #158 didn't actually fix the root of the problem. After more investigation, it seems the issue is related to objc_associatedObject API. The OBJC_ASSOCIATION_ASSIGN does not weakify references. More details about the issue can be found here: https://stackoverflow.com/questions/16569840/using-objc-setassociatedobject-with-weak-references

How to reproduce

This can actually be easily reproducible on a Playground:

import UIKit

private let _delegateKey = malloc(4)

@objc protocol SwiftyGifDelegate: AnyObject { }
class SomeObject: SwiftyGifDelegate { }

extension UIImageView {
    var delegate: SwiftyGifDelegate? {
        get { return objc_getAssociatedObject(self, _delegateKey!) as? SwiftyGifDelegate }
        set { objc_setAssociatedObject(self, _delegateKey!, newValue, .OBJC_ASSOCIATION_ASSIGN) }
    }
}

let imageView = UIImageView()

autoreleasepool {
    let object = SomeObject()
    imageView.delegate = object
}

debugPrint(imageView.delegate)

Calling the debugPrint(imageView.delegate) will crash.

Solution

Introduce a objc_setAssociatedWeakObject and objc_getAssociatedWeakObject to avoid the crash. Instead of using the OBJC_ASSOCIATION_ASSIGN with the pointer directly, we use a block that stores a weak reference, and then use OBJC_ASSOCIATION_COPY for the block.

On the playground, if you replace the previous methods with the new ones, you will see that it doesn't crash anymore when accessing the nil delegate.

@alexiscreuzot alexiscreuzot merged commit 3ea6f65 into alexiscreuzot:master Dec 16, 2021
@alexiscreuzot
Copy link
Owner

thanks for your PR

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

Successfully merging this pull request may close these issues.

2 participants