Skip to content

Commit 78f62f6

Browse files
committed
Avoid retaining objects that are being deallocated.
We do not want to retain objects that are being deallocated because this will cause a crash later when the retaining object releases them. An example of this is when a delegate (that is mocked) is called in a dealloc for an object. The mock retains the deallocating object as part of an invocation and then things crash later when the invocation is released as part of the mock deallocating itself.
1 parent 90693d3 commit 78f62f6

File tree

5 files changed

+66
-11
lines changed

5 files changed

+66
-11
lines changed

Source/OCMock/NSInvocation+OCMAdditions.m

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
#import "NSInvocation+OCMAdditions.h"
2121
#import "OCMFunctionsPrivate.h"
2222
#import "NSMethodSignature+OCMAdditions.h"
23-
23+
#import "NSObject+OCMAdditions.h"
2424

2525
#if (TARGET_OS_OSX && (!defined(__MAC_10_10) || __MAC_OS_X_VERSION_MIN_REQUIRED < __MAC_10_10)) || \
2626
(TARGET_OS_IPHONE && (!defined(__IPHONE_8_0) || __IPHONE_OS_VERSION_MIN_REQUIRED < __IPHONE_8_0))
@@ -30,7 +30,6 @@ static BOOL OCMObjectIsClass(id object) {
3030
#define object_isClass OCMObjectIsClass
3131
#endif
3232

33-
3433
@implementation NSInvocation(OCMAdditions)
3534

3635
+ (NSInvocation *)invocationForBlock:(id)block withArguments:(NSArray *)arguments
@@ -66,7 +65,7 @@ - (void)retainObjectArgumentsExcludingObject:(id)objectToExclude
6665
NSMutableArray *retainedArguments = [[NSMutableArray alloc] init];
6766

6867
id target = [self target];
69-
if((target != nil) && (target != objectToExclude) && !object_isClass(target))
68+
if((target != nil) && (target != objectToExclude) && !object_isClass(target) && ![target _isDeallocating])
7069
{
7170
// Bad things will happen if the target is a block since it's not being
7271
// copied. There isn't a very good way to tell if an invocation's target
@@ -84,7 +83,7 @@ - (void)retainObjectArgumentsExcludingObject:(id)objectToExclude
8483
{
8584
id argument;
8685
[self getArgument:&argument atIndex:index];
87-
if((argument != nil) && (argument != objectToExclude))
86+
if((argument != nil) && (argument != objectToExclude) && ![argument _isDeallocating])
8887
{
8988
if(OCMIsBlockType(argumentType))
9089
{
@@ -106,7 +105,7 @@ - (void)retainObjectArgumentsExcludingObject:(id)objectToExclude
106105
{
107106
id returnValue;
108107
[self getReturnValue:&returnValue];
109-
if((returnValue != nil) && (returnValue != objectToExclude))
108+
if((returnValue != nil) && (returnValue != objectToExclude) && ![returnValue _isDeallocating])
110109
{
111110
if(OCMIsBlockType(returnType))
112111
{

Source/OCMock/NSObject+OCMAdditions.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,9 @@
2222
+ (void)enumerateMethodsInClass:(Class)aClass usingBlock:(void (^)(Class cls, SEL sel))aBlock;
2323

2424
@end
25+
26+
27+
@interface NSObject (Internals)
28+
// Return YES if an object is in the process of being deallocated.
29+
- (BOOL)_isDeallocating;
30+
@end

Source/OCMock/OCMStubRecorder.m

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#import "OCMBlockCaller.h"
2525
#import "OCMRealObjectForwarder.h"
2626
#import "OCMInvocationStub.h"
27+
#import "NSObject+OCMAdditions.h"
2728

2829

2930
@implementation OCMStubRecorder
@@ -51,7 +52,7 @@ - (OCMInvocationStub *)stub
5152
- (id)andReturn:(id)anObject
5253
{
5354
id action;
54-
if(anObject == mockObject)
55+
if(anObject == mockObject || [anObject _isDeallocating])
5556
{
5657
action = [[[OCMNonRetainingObjectReturnValueProvider alloc] initWithValue:anObject] autorelease];
5758
}

Source/OCMockTests/OCMockObjectPartialMocksTests.m

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -608,5 +608,4 @@ - (void)testMethodSwizzlingWorksForVoidReturns
608608
XCTAssertNoThrow([foo method1], @"Should have worked.");
609609
}
610610

611-
612611
@end

Source/OCMockTests/OCMockObjectRuntimeTests.m

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,15 @@ - (NSString *)stringForTypedef:(TypedefString *)string
5454

5555
@interface TestDelegate : NSObject
5656

57-
- (void)go;
57+
- (id)go:(id)sender;
5858

5959
@end
6060

6161
@implementation TestDelegate
6262

63-
- (void)go
63+
- (id)go:(id)sender
6464
{
65+
return sender;
6566
}
6667

6768
@end
@@ -77,7 +78,38 @@ @implementation TestClassWithDelegate
7778
- (void)run
7879
{
7980
TestDelegate *delegate = self.delegate;
80-
[delegate go];
81+
[delegate go:nil];
82+
}
83+
84+
@end
85+
86+
87+
@interface TestClassThatCallsDelegateOnDealloc : NSObject
88+
89+
@property (nonatomic, weak) TestDelegate *delegate;
90+
91+
@end
92+
93+
@implementation TestClassThatCallsDelegateOnDealloc
94+
95+
- (void)dealloc
96+
{
97+
TestDelegate *delegate = self.delegate;
98+
[delegate go:self];
99+
}
100+
101+
@end
102+
103+
104+
@interface TestClassThatCallsMockWithSelfAsReturnInDealloc : NSObject
105+
@end
106+
107+
@implementation TestClassThatCallsMockWithSelfAsReturnInDealloc
108+
109+
- (void)dealloc
110+
{
111+
id newMock = [OCMockObject mockForClass:[NSString class]];
112+
[[[newMock stub] andReturn:self] uppercaseString];
81113
}
82114

83115
@end
@@ -267,7 +299,7 @@ - (void)testWeakReferencesShouldStayAround
267299

268300
[object run];
269301

270-
OCMVerify([mockDelegate go]);
302+
OCMVerify([mockDelegate go:nil]);
271303
XCTAssertNotNil(object.delegate, @"Should still have delegate");
272304
}
273305

@@ -283,6 +315,24 @@ - (void)testDynamicSubclassesShouldBeDisposed
283315
XCTAssertEqual(numClassesBefore, numClassesAfter, @"Should have disposed dynamically generated classes.");
284316
}
285317

318+
- (void)testHandlesCallingMockWithSelfAsArgumentInDealloc
319+
{
320+
// Note that this test will crash on failure.
321+
id mock = [OCMockObject mockForClass:[TestDelegate class]];
322+
[[mock expect] go:OCMOCK_ANY];
323+
TestClassThatCallsDelegateOnDealloc *foo = [[TestClassThatCallsDelegateOnDealloc alloc] init];
324+
foo.delegate = mock;
325+
foo = nil;
326+
[mock verify];
327+
}
328+
329+
- (void)testHandlesCallingMockWithSelfAsReturnInDealloc
330+
{
331+
// Note that this test will crash on failure.
332+
TestClassThatCallsMockWithSelfAsReturnInDealloc *foo = [[TestClassThatCallsMockWithSelfAsReturnInDealloc alloc] init];
333+
foo = nil;
334+
}
335+
286336

287337
#pragma mark verify mocks work properly when mocking init
288338

0 commit comments

Comments
 (0)