erikdoe / ocmock

Mock objects for Objective-C
http://ocmock.org
Apache License 2.0
2.16k stars 606 forks source link

Crash on Xcode 9 when mocking a NSManagedObject #339

Closed koke closed 6 years ago

koke commented 7 years ago

I made a sample project to reproduce the crash: https://github.com/koke/ocmock-xcode9

What I know so far from debugging:

This is the recurring part of the backtrace:

    frame #92313: 0x0000000109a784af libobjc.A.dylib`lookUpImpOrForward + 451
    frame #92314: 0x0000000109a782c1 libobjc.A.dylib`lookUpImpOrNil + 20
    frame #92315: 0x0000000109a6efa3 libobjc.A.dylib`class_respondsToSelector + 37
    frame #92316: 0x000000010a5b94ac CoreFoundation`___forwarding___ + 492
    frame #92317: 0x000000010a5b9238 CoreFoundation`__forwarding_prep_0___ + 120
    frame #92318: 0x0000000109a6ec61 libobjc.A.dylib`_class_resolveMethod + 124
    frame #92319: 0x0000000109a784af libobjc.A.dylib`lookUpImpOrForward + 451
    frame #92320: 0x0000000109a782c1 libobjc.A.dylib`lookUpImpOrNil + 20
    frame #92321: 0x0000000109a6efa3 libobjc.A.dylib`class_respondsToSelector + 37
    frame #92322: 0x000000010a5b94ac CoreFoundation`___forwarding___ + 492
    frame #92323: 0x000000010a5b9238 CoreFoundation`__forwarding_prep_0___ + 120
    frame #92324: 0x0000000109a6ec61 libobjc.A.dylib`_class_resolveMethod + 124
    frame #92325: 0x0000000109a784af libobjc.A.dylib`lookUpImpOrForward + 451
    frame #92326: 0x0000000109a78251 libobjc.A.dylib`class_getInstanceMethod + 53
  * frame #92327: 0x00000001211fd2f9 ocmock-xcode9-test`-[OCClassMockObject setupForwarderForClassMethodSelector:](self=0x0000618001060140, _cmd="setupForwarderForClassMethodSelector:", selector="retain") at OCClassMockObject.m:149
imhuntingwabbits commented 7 years ago

Don't mock NSManagedObject. It will never work.

maxfriedrich commented 7 years ago

Mocking managed objects is actually a feature since OCMock v3.4. It seems fine with Xcode 9 when a iOS 10 or any macOS SDK is selected, it only breaks on iOS 11. I ran the OCMockTests on an iOS 11 simulator and they crash as well (-[OCMockObjectPartialMocksTests testMockingManagedObject]) with the same stack trace.

Any ideas on this? We use mogenerator and have custom logic in our NSManagedObject subclasses so there aren't any protocols that we can mock instead of the managed objects.

DanieleMoonPig commented 7 years ago

I have some old test that are using PartialMock on a NSManagedObject, on iOS 11 I have the same problem. Exactly like @koke described. We have used the ManagedObject Generator too and no protocols, this migration will cost me a lot of time! My solution for now is remove the OCMPartialMock on ManagedObjects and create real mocks.

Kieran2k15 commented 7 years ago

Same problem, suddenly started crashing on XCode 9 on iOS 11.

YuriSolodkin commented 7 years ago

I have the same issue

sergiobaro commented 7 years ago

I have the same problem. Xcode 9 on iOS 11.

adavalli123 commented 7 years ago

Same problem :(

erikdoe commented 7 years ago

Thank you. I am aware of the problem. Haven't had a chance to look into it in detail. Please do not add more "me too" comments.

givip commented 7 years ago

It's seems that bug not depend on Xcode 9. Problem is in iOS 11, on iOS 10 simulator in Xcode 9 all works.

erikdoe commented 7 years ago

So, I have looked into this but haven't really understood why this happens with the updated runtime in iOS 11.

That said, I did find a workaround. A way to stop the crashes is to avoid installing the method hooks on class methods of NSManagedObject. This means, though, that verification of these methods becomes impossible. For those of you who use OCMock with NSManagedObject, is this reasonable?

In any case, in a moment I will push a commit that implements this workaround. Please let me know what you think of it.

erikdoe commented 6 years ago

Can you please confirm whether this fix works for you? If it does it'd do a small release.

givip commented 6 years ago

I'll check only on Monday. Thanks.

Отправлено с iPhone

21 окт. 2017 г., в 22:15, Erik Doernenburg notifications@github.com написал(а):

Can you please confirm whether this fix works for you? If it does it'd do a small release.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

murthyveda2000 commented 6 years ago

It did not work for me. I am still getting a crash.

35207 0x00000001372f7ce2 in -[OCClassMockObject setupForwarderForClassMethodSelector:] at OCClassMockObject.m:151

35208 0x00000001372f7c19 in __54-[OCClassMockObject prepareClassForClassMethodMocking]_block_invoke at OCClassMockObject.m:139

35209 0x00000001372f71d3 in +[NSObject(OCMAdditions) enumerateMethodsInClass:usingBlock:] at /OCMock/NSObject+OCMAdditions.m:69

35210 0x00000001372f7ac4 in -[OCClassMockObject prepareClassForClassMethodMocking] at OCClassMockObject.m:125

35211 0x00000001372f7662 in -[OCClassMockObject initWithClass:] at OCClassMockObject.m:33

35212 0x00000001372fe0f2 in -[OCPartialMockObject initWithObject:] at OCPartialMockObject.m:35

35213 0x00000001372fb0d4 in +[OCMockObject partialMockForObject:] at OCMockObject.m:59

seanhenry commented 6 years ago

This workaround fixed the issue for me

erikdoe commented 6 years ago

@murthyveda2000 When you say you're still getting a crash, do you mean the code sample from the opening post crashes? Or do you have a different test that crashes? If so, could you provide more detail?

erikdoe commented 6 years ago

In the meantime I released OCMock 3.4.1 which contains the workaround.

FlorianMielke commented 6 years ago

3.4.1 does not fix it for me. Still getting crash.

Applitom commented 6 years ago

Hi @erikdoe, I think the other crashes happens when we try to mock an object that inherits from NSManagedObject. Please see how I implemented your workaround to support this scenario: https://github.com/erikdoe/ocmock/compare/master...applitom:master

Thanks, Tomer

imhuntingwabbits commented 6 years ago

@Applitom am I reading your change correctly? It looks like you're attempting to mock methods on your classes before CoreData has scribbled them in to the runtime?

givip commented 6 years ago

@erikdoe Not working for PartialMock, in case of following initialisation OCMPartialMock([[ManagedObject alloc] initWithEntity:entity insertIntoManagedObjectContext:context]); I think that better workaround will be replacement NSManagedObject metaClass to NSObject metaClass.

imhuntingwabbits commented 6 years ago

@givip why do you need a partial mock of a managed object?

Swizzling the metaClass won't help races on selectors that don't exist in the runtime yet.

imhuntingwabbits commented 6 years ago

@erikdoe FWIW I'm not sure what OCMock is going to do here. As far as I can tell these issues are based on races with the initialization of the CoreData stack, which is a private implementation detail to the framework.

In fact some classes / selectors aren't written in to the runtime until the objects are used by an instance of NSManagedObjectContext. Without materializing a full stack in memory "this will never work".

The fact that this changed across major OS releases isn't surprising, and any test code that functions today given a certain initialization loop is inherently subject to failure in future releases as they permutations of loop change for future optimizations.

erikdoe commented 6 years ago

@Applitom Thanks for looking into it. Do you have a test case that fails without your change but passes with it? I'm asking because as far as I can tell, your change only affects class methods added by subclasses of NSManagedObject.

It might not be totally obvious but the change I made does already affect subclasses of NSManagedObjects. This is because enumerateMethodsInClass:usingBlock: walks up the superclass chain, and the cls argument passed to the block is the class in the hierarchy that actually implements the method.

Example: assume you have a class Foo that has a class method bar. The block will be called with class Foo and method bar and in that case your change prevents a forwarder to be implemented. As far as class methods implemented by NSManagedObject go, the block will be called with NSManagedObject (and not the subclass that is being mocked), and therefore those class methods will be skipped even without your change.

erikdoe commented 6 years ago

@murthyveda2000, @FlorianMielke, @givip, do all of you encounter the crash only when you try to use a partial mock with a managed object?

FlorianMielke commented 6 years ago

Yes.

rt-bink commented 6 years ago

@erikdoe I've been updating our companies project to iOS 11 and Xcode 9. As a result, I've also come into contact with this issue. I've updated the pod spec to use 3.4.1 and the above issue presents when using a partial mock on a managed object.

erikdoe commented 6 years ago

@rt-bink Thanks for providing another data point. I think, at this stage, it's pretty clear that the change included in OCMock 3.4.1 fixes most of the crashes with NSManagedObject that were caused by the changes in the new runtime, leaving the crash with partial mocks for NSManagedObject the only open part of this issue.

To be honest, at the moment I don't know how to change OCMock so that it can still create partial mocks for NSManagedObject with Apple's changes in the new runtime. Not saying it won't happen, but it will probably take some time. If you have any ideas please post them here.

PierreEc commented 6 years ago

@erikdoe OCMock 3.4.1 doesn't fix the crash with NSManagedObject for me. However, the implementation of your workaround suggest by @Applitom correct it

reni99 commented 6 years ago

Hi there So do you plan to merge the workaround suggest by @Applitomthis into master too? Thx!

erikdoe commented 6 years ago

As I wrote in my comment on 11 Nov, I don't understand what the proposed change does. The fix I added should cover subclasses. As I wrote back then: Do you have a test case that fails without your change but passes with it? Did I miss a comment somewhere?

PierreEc commented 6 years ago

@erikdoe, as I told you on 22 Feb, the workaround suggest by @Applitom correct the problem for me, but not your workaround. To reproduce it, just use partialMock for a NSManagedObject.

bochos-bln commented 6 years ago

Hi, i had the same problem with a crash for partialMockForObject with OCMock 3.4.1.

Adding the changes from https://github.com/erikdoe/ocmock/pull/354/commits/9d2a803dedfe625983684fa7b2b78a2d241f62b8 fixed the crash and the (old) test seem also to work.

Please add this to a new Release

erikdoe commented 6 years ago

I hear what you say. Would it be possible, though, please, to provide a unit test that fails without that change and passes with that change? I would really like to understand the problem.

Applitom commented 6 years ago

Hello, I'm sorry about the late response (better late than never 😄). I made a little example of the crash, I hope it explains our problem better. You can find it here: https://github.com/Applitom/ocmock-issue339-fix

Thanks!

erikdoe commented 6 years ago

Thank you for providing the example; very helpful. I now had time and a machine with Xcode 9.3 (don't ask) to look into this. Finally, it's clear to me what the underlying problem is.

The change I just made fixes the problem and it should have less of a performance impact than the original proposal. A known downside of the way I addressed it is that you cannot stub/mock class methods on NSObject for subclasses of NSManagedObject.

Could you let me know whether that works for you?

Applitom commented 6 years ago

Hi @erikdoe, Thanks, I can confirm that now it's working for me when I point to master branch. could you please release new version with this fix?

Thanks, Tomer.

erikdoe commented 6 years ago

Thanks for confirming. I'll get a release out when I have a moment, hopefully on Monday.

erikdoe commented 6 years ago

Release is out on Github, Carthage, and Cocoapods. Website will be a bit delayed.