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

Make it safe to use -mkt_arguments as a generic helper #175

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

0x1eaf
Copy link

@0x1eaf 0x1eaf commented Dec 18, 2024

Currently it is not safe to use -[NSInvocation mkt_arguments] with NSInvocation objects that don't come from OCMockito itself, as it doesn't call -[NSInvocation retainArguments] before adding the argument values to an autoreleased NSArray. Meaning that if the NSInvocation object has a stack block argument, it would be added as is to the NSArray as -[__NSStackBlock__ retain] does nothing. And the program might crash if the autorelease elision optimisation happens to not be performed by the objc runtime on the array, and the array ends up in an autorelease pool outliving the stack block and segfaults when trying to release the block during deallocation when the pool is finally popped.

It is safe to use -mkt_arguments with NSInvocation objects produced by OCMockito itself as -[NSInvocation retainArguments] is automatically called via:

  • -[NSInvocation mkt_retainArgumentsWithWeakTarget]
  • -[MKTInvocationContainer setInvocationForPotentialStubbing:]
  • -[MKTBaseMockObject forwardInvocation:]

It is trivial, safe and cheap to add a call to -retainArguments at the start of -mkt_arguments as it doesn't do anything in case if the arguments have already been retained, which is the case for NSInvocation objects produced by OCMockito itself.

And this makes it safe and convenient to use -mkt_arguments as a generic helper for any NSInvocation objects regardless of their provenance.

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.

1 participant