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

NSInvocation+OCMAdditions: don't retain deallocating objects. #11

Merged
merged 1 commit into from
May 22, 2022
Merged

NSInvocation+OCMAdditions: don't retain deallocating objects. #11

merged 1 commit into from
May 22, 2022

Conversation

ashdnazg
Copy link
Member

Solves erikdoe#346.
allowsWeakReference returns NO if the receiver is during
deallocation. Not retaining deallocating objects would prevent crashing
later when the retained arguments are released - including the
previously deallocated objects.
There are some classes that can never have weak references - these would
also not be retained which might limit some OCMock functionality.

@byohay
Copy link

byohay commented Nov 28, 2021

@ashdnazg @barakwei what about this?

@ashdnazg ashdnazg changed the base branch from test-master to master May 9, 2022 08:32
@barakwei barakwei removed their assignment May 12, 2022
Copy link

@byohay byohay left a comment

Choose a reason for hiding this comment

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

Looks good! Check that tests pass in Foundations and merge.

@@ -66,7 +66,8 @@ - (void)retainObjectArgumentsExcludingObject:(id)objectToExclude
NSMutableArray *retainedArguments = [[NSMutableArray alloc] init];

id target = [self target];
if((target != nil) && (target != objectToExclude) && !object_isClass(target))
if((target != nil) && (target != objectToExclude) && !object_isClass(target) &&
[target allowsWeakReference])
Copy link

Choose a reason for hiding this comment

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

I'd add a comment as to why allowsWeakReference is needed.

Solves erikdoe#346.
`allowsWeakReference` returns `NO` if the receiver is during
deallocation. Not retaining deallocating objects would prevent crashing
later when the retained arguments are released - including the
previously deallocated objects.
There are some classes that can never have weak references - these would
also not be retained which might limit some OCMock functionality.
@ashdnazg ashdnazg merged commit 5fe0c6b into Lightricks:master May 22, 2022
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

Successfully merging this pull request may close these issues.

3 participants