1

I'm trying to mock a call to mutableCopy using OCMock and GHUnit on iOS.

Despite the test passing, I get an EXC_BAD_ACCESS exception during the cleanup, and I'm trying to work out why.

Take a look at this. This test shows that it is possible to mock mutableCopy on a mock NSString. In this test I return another NSString, not an NSMutableString. This is just to demonstrate that the mutableCopy expectation is fired, and the test passes.

#import <GHUnitIOS/GHUnit.h>
#import "OCMock.h"

@interface TestItClass : GHTestCase @end
@implementation TestItClass

// Test that mutableCopy on an NSString is mockable.
- (void)test_1_mutableCopyOfString_shouldBeMockable_givenAStringIsReturned {
    NSString *string = [OCMockObject mockForClass:NSString.class];
    NSString *copy = @"foo";
    [(NSString *) [[(id) string expect] andReturn:copy] mutableCopy];

    // MutableCopy is mocked to return a string, not a mutable string!
    // This is clearly wrong from a static typing point of view, but
    // the test passes anyway, which is ok.
    NSMutableString *result = [string mutableCopy];
    GHAssertEquals(result, copy, nil);
    [(id)string verify];
}

Now I change the mock expectation so that mutableCopy now returns an NSMutableString. The test still passes, but on the tear down of the test I get a EXC_BAD_ACCESS exception.

- (void)test_2_mutableCopyOfString_shouldBeMockable_givenAMutableStringIsReturned {
    NSString *string = [OCMockObject mockForClass:NSString.class];
    NSMutableString *copy = [@"foo" mutableCopy];
    [(NSString *) [[(id) string expect] andReturn:copy] mutableCopy];

    // Now mutableCopy is mocked to return a mutable string!
    // The test now blows up during the test teardown! Why?
    NSMutableString *foo = [string mutableCopy];
    GHAssertEquals(foo, copy, nil);
    [(id)string verify];
}

@end

In both tests the verifies work, as to the asserts. This shows that both tests are well constructed and that the mock expectations are being fired as expected. However, the second test fails in the tear down with a bad memory access:

Simulator session started with process 7496
Debugger attached to process 7496
2013-03-11 18:23:05.519 UnitTests[7496:c07] TestItClass/test_2_mutableCopyOfString_shouldBeMockable_givenAMutableStringIsReturned ✘ 0.00s
2013-03-11 18:23:06.466 UnitTests[7496:c07] Re-running: TestItClass/test_2_mutableCopyOfString_shouldBeMockable_givenAMutableStringIsReturned <GHTest: 0x7793340>
Exception: EXC_BAD_ACCESS (code=1, address=0x11dfe3ea))

Can you please suggest to me why it might be happening?

Thanks, Joe

Dr Joe
  • 718
  • 5
  • 19
  • Please include your `- (void)tearDown` methord in the question – Shineeth Hamza Mar 12 '13 at 06:30
  • There isn't an explicit tearDown; it's just the stock one that GHUnit provides. – Dr Joe Mar 12 '13 at 11:39
  • @Shineeth Actually, it looks like the crash is in the teardown of one of the OCMock classes. I've posted part of the answer below. – Dr Joe Mar 12 '13 at 12:28
  • Can you explain what you're trying to achieve in this test? There usually isn't any reason to mock core API methods that have well-defined behavior. There's probably a better way to achieve what you want to test. – Christopher Pickslay Mar 12 '13 at 15:46
  • Hi Christopher, sure, and I agree. Unfortunately in this case there is a fairly sophisticated output string being generated from the input, in relation to a small number of external well-tested builder classes. I don't want the implementation details of these to be a consideration of the test of the class that uses them. That requires that I mock their interactions with the mutable string which is accumulating their effects. Not ideal, but I'm unwilling to give up the encapsulation of concerns. – Dr Joe Mar 12 '13 at 18:25
  • @Christopher Despite questions of ideal test construction, there shouldn't be any technical reason why this kind of test shouldn't work, right? – Dr Joe Mar 12 '13 at 18:36

3 Answers3

2

the problem your facing is caused by the fact that ARC follows the Basic Memory Management Rules. Specifically this:

  • You own any object you create

    You create an object using a method whose name begins with “alloc”, “new”, “copy”, or “mutableCopy” (for example, alloc, newObject, or mutableCopy).

So the solution would be to look at the invocation selector to determine whether to retain the returnValue or not.

I hope this help.

  • This is the bit that I was missing. I'm glad that we've found the bug in OCMock, and the maintainer has fixed it as a consequence. Thanks :). – Dr Joe Mar 14 '13 at 20:48
  • 1
    The explanation provided by Zaygraveyard is right; and OCMock did not behave properly for methods that create objects. I have now implemented a solution. See https://github.com/erikdoe/ocmock/commit/00c0fe362de2b11625d348ae50379a108a3abf0b. – Erik Doernenburg Mar 14 '13 at 15:38
0

I'm part way to understanding what is going on. I've compiled myself a debug library of OCMock so that I can understand where the crash is occurring.

Here's what I've found.

In my original test I call andReturn: to set a return expectation value:

NSMutableString *copy = [@"foo" mutableCopy];
[(NSString *) [[(id) string expect] andReturn:copy] mutableCopy];

This in turn calls OCMReturnValueProvider to store a copy so that it can returned at the appropriate time:

@implementation OCMReturnValueProvider

- (id)initWithValue:(id)aValue
{
    self = [super init];
    returnValue = [aValue retain];
    return self;
}

At this point the debugger says that aValue is of type __NSCFString. (Alarm bells ring in my head; isn't that a toll free bridge to an underlying string? Not a reference to the NSMutableString)

Next the test completes and passes.

However, the problem now occurs when the OCMReturnValueProvider is dealloc'd.

@implementation OCMReturnValueProvider
- (void)dealloc
{
    [returnValue release];
    [super dealloc];
}

The crash happens when [returnValue release] is called; the OCMReturnValueProvider is trying to release the __NSCFString that it retained earlier.

Next, I've switch on NSZombie debugging, which is revealing:

2013-03-12 20:58:19.654 UnitTests[16667:c07] TestItClass/test_2_mutableCopyOfString_shouldBeMockable_givenAMutableStringIsReturned  
2013-03-12 20:58:21.778 UnitTests[16667:c07] Re-running: TestItClass/test_2_mutableCopyOfString_shouldBeMockable_givenAMutableStringIsReturned <GHTest: 0x4afc5fd0>
2013-03-12 20:58:21.780 UnitTests[16667:c07] *** -[CFString release]: message sent to deallocated instance 0x4b0b1fe0

The malloc-history (Find Zombie instrument) is helping shed some light on it:

Category            Event Type  Ref Ct  Responsible Caller
CFString (mutable)  Malloc        1     -[TestItClass test_2_mutable...]
CFString (mutable)  Retain        2     -[OCMReturnValueProvider initWithValue:]
CFString (mutable)  Retain        3     -[TestItClass test_2_mutable...]
CFString (mutable)  Retain        4     -[TestItClass test_2_mutable...]
CFString (mutable)  Release       3     -[TestItClass test_2_mutable...]
CFString (mutable)  Release       2     -[TestItClass test_2_mutable...]
CFString (mutable)  Release       1     -[TestItClass test_2_mutable...]
CFString (mutable)  Release       0     -[TestItClass test_2_mutable...]
CFString (mutable)  Zombie       -1     -[OCMReturnValueProvider dealloc]

So something in the test class is causing more releases than retains. Why's that happening? Strange!

Dr Joe
  • 718
  • 5
  • 19
0

After a bit more investigation I've discovered why the crash is occurring.

Let's look at the test again:

- (void)test_2_mutableCopyOfString_shouldBeMockable_givenAMutableStringIsReturned {
    NSString *string = [OCMockObject mockForClass:NSString.class];
    NSMutableString *copy = [@"foo" mutableCopy];
    [(NSString *) [[(id) string expect] andReturn:copy] mutableCopy];

    NSMutableString *foo = [string mutableCopy];
}

What is happening is that the compiler is assuming that the object returned by [string mutableCopy] is was retained by mutableCopy, and so when foo is dealloced ARC does the equivalent of [foo release]. This is a problem because we this object didn't have it's reference count increased within andReturn:.

I am confused as to why we don't see this behaviour with other objects configured to be returned by andReturn:. The OCMReturnValueProvider handling the mocked response is not ARC managed, and isn't retaining the returned value:

- (void)handleInvocation:(NSInvocation *)anInvocation
{
    [anInvocation setReturnValue:&returnValue];
}

So, the problem is simply solved by pre-emptively retaining the returned value before setting it in the NSInvocation:

- (void)handleInvocation:(NSInvocation *)anInvocation
{
    [returnValue retain];
    [anInvocation setReturnValue:&returnValue];
}

This looks like a bug in OCMock. But given that problem doesn't occur in all circumstances, I'm not sure exactly. My fix works, but now runs the risk of leaking memory on objects which might not need this extra retain. However, a memory leak in a test, vs a test that doesn't run, is acceptable to me for now.

Dr Joe
  • 718
  • 5
  • 19
  • I've proposed this fix to the maintainer of OCMock. Will report back here once it's been resolved. – Dr Joe Mar 14 '13 at 07:44
  • I believe the problem is that mutableCopy is somewhat special. Normally it's the responsibility of the caller to retain objects that they want to keep around. The called method returns the object with a "neutral" retain count. MutableCopy however returns an object with a +1 retain count. I saw the pull request but will try to find a solution that doesn't leak memory. – Erik Doernenburg Mar 14 '13 at 10:42
  • The explanation provided by Zaygraveyard is right; and OCMock did not behave properly for methods that create objects. I have now implemented a solution. See https://github.com/erikdoe/ocmock/commit/00c0fe362de2b11625d348ae50379a108a3abf0b. – Erik Doernenburg Mar 14 '13 at 15:37