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

Failure to remove weak reference when objc_storeWeak() is called with nil object #311

Closed
rfm opened this issue Nov 16, 2024 · 2 comments
Closed
Assignees

Comments

@rfm
Copy link
Contributor

rfm commented Nov 16, 2024

Storing a nil value to a weak reference is, by my reading of the documentation, supposed to remove the weak reference.
Mostly that's what objc_storeWeak() does, but there is a situation where it doesn't do that:

Consider this sequence of events:

an object is stored to an address using objc_storeWeak(), creating a weak reference
the object is then deallocated, leading to the pointer being zeroed, but the runtime still keeping track of the address
then we want to stop using that address as a weak reference, so we use objc_storeWeak() to store null to it
however, because the old value and the new value are both zero, the existing implementation doesn't remove the weak reference.

The code looks like this:
// If the old and new values are the same, then we don't need to do anything.
if (old == obj)
{
return obj;
}
perhaps it should be
// If the old and new values are the same, then we don't need to do anything.
if (obj != NULL && old == obj)
{
return obj;
}

@rfm
Copy link
Contributor Author

rfm commented Nov 18, 2024

I'm fairly confident my reading of what this function should do is correct, and the code change obviously fixes it to avoid leaking reference information (for code in the base library which assumes that we can call it to delete a weak reference).
If so, I guess the associated comment would need to be improved too.

@rfm
Copy link
Contributor Author

rfm commented Dec 20, 2024

I made a pull request for this instead.

@rfm rfm closed this as completed Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants