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

Crash on Xcode 9 when mocking a NSManagedObject #339

Closed
koke opened this Issue Jun 9, 2017 · 38 comments

Comments

Projects
None yet
@koke

koke commented Jun 9, 2017

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

What I know so far from debugging:

  • setupForwarderForClassMethodSelector is called successfully for a few selectors
  • setupForwarderForClassMethodSelector is called successfully for resolveClassMethod:
  • setupForwarderForClassMethodSelector is called for retain, then it enters infinite recursion and eventually crashes

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

This comment has been minimized.

Show comment
Hide comment
@imhuntingwabbits

imhuntingwabbits Jun 9, 2017

Contributor

Don't mock NSManagedObject. It will never work.

Contributor

imhuntingwabbits commented Jun 9, 2017

Don't mock NSManagedObject. It will never work.

@maxfriedrich

This comment has been minimized.

Show comment
Hide comment
@maxfriedrich

maxfriedrich Jul 19, 2017

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.

maxfriedrich commented Jul 19, 2017

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

This comment has been minimized.

Show comment
Hide comment
@DanieleMoonPig

DanieleMoonPig Jul 24, 2017

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.

DanieleMoonPig commented Jul 24, 2017

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

This comment has been minimized.

Show comment
Hide comment
@Kieran2k15

Kieran2k15 Sep 18, 2017

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

Kieran2k15 commented Sep 18, 2017

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

@YuriSolodkin

This comment has been minimized.

Show comment
Hide comment
@YuriSolodkin

YuriSolodkin Sep 22, 2017

I have the same issue

YuriSolodkin commented Sep 22, 2017

I have the same issue

@sergiobaro

This comment has been minimized.

Show comment
Hide comment
@sergiobaro

sergiobaro Sep 27, 2017

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

sergiobaro commented Sep 27, 2017

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

@adavalli123

This comment has been minimized.

Show comment
Hide comment
@adavalli123

adavalli123 Oct 2, 2017

Same problem :(

adavalli123 commented Oct 2, 2017

Same problem :(

@erikdoe

This comment has been minimized.

Show comment
Hide comment
@erikdoe

erikdoe Oct 3, 2017

Owner

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.

Owner

erikdoe commented Oct 3, 2017

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

This comment has been minimized.

Show comment
Hide comment
@givip

givip Oct 4, 2017

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

givip commented Oct 4, 2017

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

This comment has been minimized.

Show comment
Hide comment
@erikdoe

erikdoe Oct 13, 2017

Owner

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.

Owner

erikdoe commented Oct 13, 2017

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

This comment has been minimized.

Show comment
Hide comment
@erikdoe

erikdoe Oct 21, 2017

Owner

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

Owner

erikdoe commented Oct 21, 2017

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

@givip

This comment has been minimized.

Show comment
Hide comment
@givip

givip Oct 21, 2017

givip commented Oct 21, 2017

@murthyveda2000

This comment has been minimized.

Show comment
Hide comment
@murthyveda2000

murthyveda2000 Oct 23, 2017

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

murthyveda2000 commented Oct 23, 2017

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

This comment has been minimized.

Show comment
Hide comment
@seanhenry

seanhenry Oct 31, 2017

This workaround fixed the issue for me

seanhenry commented Oct 31, 2017

This workaround fixed the issue for me

@erikdoe

This comment has been minimized.

Show comment
Hide comment
@erikdoe

erikdoe Nov 5, 2017

Owner

@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?

Owner

erikdoe commented Nov 5, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@erikdoe

erikdoe Nov 5, 2017

Owner

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

Owner

erikdoe commented Nov 5, 2017

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

@FlorianMielke

This comment has been minimized.

Show comment
Hide comment
@FlorianMielke

FlorianMielke Nov 6, 2017

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

FlorianMielke commented Nov 6, 2017

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

@Applitom

This comment has been minimized.

Show comment
Hide comment
@Applitom

Applitom Nov 6, 2017

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:
master...applitom:master

Thanks,
Tomer

Applitom commented Nov 6, 2017

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:
master...applitom:master

Thanks,
Tomer

@imhuntingwabbits

This comment has been minimized.

Show comment
Hide comment
@imhuntingwabbits

imhuntingwabbits Nov 6, 2017

Contributor

@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?

Contributor

imhuntingwabbits commented Nov 6, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@givip

givip Nov 7, 2017

@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.

givip commented Nov 7, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@imhuntingwabbits

imhuntingwabbits Nov 7, 2017

Contributor

@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.

Contributor

imhuntingwabbits commented Nov 7, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@imhuntingwabbits

imhuntingwabbits Nov 7, 2017

Contributor

@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.

Contributor

imhuntingwabbits commented Nov 7, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@erikdoe

erikdoe Nov 11, 2017

Owner

@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.

Owner

erikdoe commented Nov 11, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@erikdoe

erikdoe Nov 11, 2017

Owner

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

Owner

erikdoe commented Nov 11, 2017

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

@FlorianMielke

This comment has been minimized.

Show comment
Hide comment
@FlorianMielke

FlorianMielke commented Nov 11, 2017

Yes.

@rt-bink

This comment has been minimized.

Show comment
Hide comment
@rt-bink

rt-bink Nov 24, 2017

@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.

rt-bink commented Nov 24, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@erikdoe

erikdoe Nov 24, 2017

Owner

@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.

Owner

erikdoe commented Nov 24, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@PierreEc

PierreEc Feb 22, 2018

@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

PierreEc commented Feb 22, 2018

@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

This comment has been minimized.

Show comment
Hide comment
@reni99

reni99 Apr 4, 2018

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

reni99 commented Apr 4, 2018

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

@erikdoe

This comment has been minimized.

Show comment
Hide comment
@erikdoe

erikdoe Apr 5, 2018

Owner

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?

Owner

erikdoe commented Apr 5, 2018

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

This comment has been minimized.

Show comment
Hide comment
@PierreEc

PierreEc Apr 6, 2018

@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.

PierreEc commented Apr 6, 2018

@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.

@xlbs-rm

This comment has been minimized.

Show comment
Hide comment
@xlbs-rm

xlbs-rm Apr 18, 2018

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

Adding the changes from 9d2a803
fixed the crash and the (old) test seem also to work.

Please add this to a new Release

xlbs-rm commented Apr 18, 2018

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

Adding the changes from 9d2a803
fixed the crash and the (old) test seem also to work.

Please add this to a new Release

@erikdoe

This comment has been minimized.

Show comment
Hide comment
@erikdoe

erikdoe Apr 20, 2018

Owner

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.

Owner

erikdoe commented Apr 20, 2018

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

This comment has been minimized.

Show comment
Hide comment
@Applitom

Applitom Apr 24, 2018

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!

Applitom commented Apr 24, 2018

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

This comment has been minimized.

Show comment
Hide comment
@erikdoe

erikdoe May 25, 2018

Owner

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?

Owner

erikdoe commented May 25, 2018

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

This comment has been minimized.

Show comment
Hide comment
@Applitom

Applitom Jun 20, 2018

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.

Applitom commented Jun 20, 2018

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

This comment has been minimized.

Show comment
Hide comment
@erikdoe

erikdoe Jun 21, 2018

Owner

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

Owner

erikdoe commented Jun 21, 2018

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

@erikdoe

This comment has been minimized.

Show comment
Hide comment
@erikdoe

erikdoe Jun 25, 2018

Owner

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

Owner

erikdoe commented Jun 25, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment