facebookarchive / FBMemoryProfiler

iOS tool that helps with profiling iOS Memory usage.
Other
3.41k stars 377 forks source link

FBMemoryProfiler message sent to deallocated crash on iOS 9.2.1 #4

Closed intoxicated closed 8 years ago

intoxicated commented 8 years ago

I was really happy that finally these kinda tool was released to catch retained cycle which have been pain in my ass for a long time. I installed through Cocoapods, followed the instruction for basic setup and had played around with it. And guess what, it couldn't stay for a minute :( .It's 100% reproducible and steps are following:

  1. open app with FBMemoryProfiler
  2. toggle profile window
  3. click expand button
  4. click retain cycles

FBRetainCycleUnits.m

FBObjectiveCGraphElement *FBWrapObjectGraphElementWithContext(id object,
                                                              FBObjectGraphConfiguration *configuration,
                                                              NSArray<NSString *> *namePath) {
  if (FBObjectIsBlock((__bridge void *)object)) {
    return [[FBObjectiveCBlock alloc] initWithObject:object
                                      configuration:configuration
                                            namePath:namePath];
  } else {
    if ([object_getClass(object) isSubclassOfClass:[NSTimer class]] &&
        configuration.shouldInspectTimers) {
      return [[FBObjectiveCNSCFTimer alloc] initWithObject:object
                                             configuration:configuration
                                                  namePath:namePath];
    } else {
      return [[FBObjectiveCObject alloc] initWithObject:object
                                          configuration:configuration
                                               namePath:namePath];
    }
  }
}

//crash here
FBObjectiveCGraphElement *FBWrapObjectGraphElement(id object,
                                                   FBObjectGraphConfiguration *configuration) {
  return FBWrapObjectGraphElementWithContext(object, configuration, nil);
}

screen shot 2016-04-14 at 10 28 02 pm

Update: I followed the call stacks and found out that it happened when mutating a collection while enumeration. The call stack right before the crash is following:

    NSInteger tries = 10;
    for (NSInteger i = 0; i < tries; ++i) {
      // If collection is mutated we want to rollback and try again - let's keep refs in temporary set
      NSMutableSet *temporaryRetainedObjects = [NSMutableSet new];
      @try {
        for (id subobject in self.object) {
          if (retainsKeys) {
           //crash here
            [temporaryRetainedObjects addObject:FBWrapObjectGraphElement(subobject, self.configuration)];
          }
          if (isKeyValued && retainsValues) {
            [temporaryRetainedObjects addObject:FBWrapObjectGraphElement([self.object objectForKey:subobject],
                                                                         self.configuration)];
          }
        }
      }
      @catch (NSException *exception) {
        // mutation happened, we want to try enumerating again
        continue;
      }

Update2: Turn on zombie objects and got following: [_UILabelLayer respondsToSelector:]: message sent to deallocated instance 0x110982130 0x0000000110982130

I guess when tableview was filled with objects and clicking retain cycle (highlighting) caused the issue due to sending message to a object which is never allocated before? It seems working fine with few items

Gricha commented 8 years ago

Thanks for reporting. I'm sorry you are having this problem.

The way the framework is constructed it gives a small leeway for crashes when running retain cycle detection on all objects, but I've never seen it being 100% reproducible.

Can you please tell me whether it's always _UILabelLayer that fails?

The trick we can also use is - you can run retain cycle detection on just one class of objects by tapping on the label inside FBMemoryProfiler.

I'll also look more into that and try to reproduce in my apps.

intoxicated commented 8 years ago

@Gricha Yes it always happened on UILabel. It won't crash if I used filter or tapping on the label inside tho. I tried to fix it myself by tracking down the bug :| Though can't find exact point to fix. I would like to contribute if I can

intoxicated commented 8 years ago

And beside this bug, there are a few broken links in README.md file and it would be helpful to explain what are those texts indicate (when you tap on label and name of class then some other labels in 2nd depth)

Gricha commented 8 years ago

I'll take care of broken links early tomorrow.

It's most interesting. Does this repro 100% of time? Are you able to build a minimal app so that it would still repro? I cannot see this behavior on common, generic UILabels.

When it comes to 2nd depth - it definitely should be presented in a better way, happy to see other ideas in this. Right now when you tap on label of leaking class you get a table view of instances of these leaks. Once tapped on one of them what you see is a representation of that leak: -> ivar -> class -> ivar -> class etc.

intoxicated commented 8 years ago

I should've mention this earlier but I tried profiler on my production application, and yes it was 100% reproducible. I didn't try with minimal app tho. Maybe it has too many UILabels? lol.. not sure. I did some digging last night and it went through much of NSObjects (non-UI related objects such as AFNetworking), but it crashed when self.object contains bunch of UI components (assumed UI of application) and attempts to generate graphElement.

martinpilch commented 8 years ago

I'm having the same issue, same scenario on iOS 9.3

Antondomashnev commented 8 years ago

First of all thanks for the great tool!

And I have the same issue on iOS 9.3

Gricha commented 8 years ago

If you have a repro - does it happen on one particular type of objects, or does it happen just randomly because of thread un-safety? We could try incorporating fix from FBAllocationTracker and try running it with NSZombies to see if it makes situation better.

raja-baz commented 8 years ago

Have this happening on my project as well. Will try to come up with a minimal app that reproduces it.

@Gricha As for whether it's always the same type of object or not. For me, 90% of the time this happens it's the same type of object, a subclass of RLMObject. We use Realm, and for now, that's the only model we have. It may be due to something particular to that class, or it may be the fact that these objects are often manipulated on background threads and thus due to some thread-unsafety issues, I cannot tell.

raja-baz commented 8 years ago

Here's a project that reproduces the issue:

TestingFBMemoryCrash.zip

To reproduce, simply install pods, fire up the workspace and run it in a simulator. Then, from the memory profiler, search for and tap SomeObject and voila:

screen shot 2016-04-20 at 4 16 29 pm
hhanesand commented 8 years ago

I've been getting this crash intermittently as well

Gricha commented 8 years ago

Thanks @raja-baz I'll look into it!

Gricha commented 8 years ago

This is a great repro, I'll work on fixing this bug. Thanks!

Gricha commented 8 years ago

Just an update on that issue, I already created a pull request to fix the issue on FBAllocationTracker, and it should be coming along pretty soon, but that fix does not work with Realm objects for some reason. I need to get deeper into why it's the case.

Besides that it works swimmingly with other types of objects and improves stability of the framework.

raja-baz commented 8 years ago

Thanks for the update. I guess Realm's ORM stuff (introspection and all the dynamic stuff they do under the hood so you have disk-backed objects that automatically update when the db stuff) is messing with the introspection stuff happening from your end.

I hope you figure this stuff out soon and thank you for this awesome project :-) On Apr 29, 2016 11:58 PM, "Grzegorz Pstrucha" notifications@github.com wrote:

Just an update on that issue, I already created a pull request to fix the issue on FBAllocationTracker, and it should be coming along pretty soon, but that fix does not work with Realm objects for some reason. I need to get deeper into why it's the case.

Besides that it works swimmingly with other types of objects and improves stability of the framework.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/facebook/FBMemoryProfiler/issues/4#issuecomment-215880179

Gricha commented 8 years ago

I have merged the fix in, I will open another issue for Realm and close out this one as the "main" problem is fixed (the more specific ones are still ongoing)

vsravan commented 7 years ago

I'm woundering how it will attache to Xcode to show leaked memory

Yincongxiao commented 5 years ago

I think this memory issue is caused by FBRetainCycleDetector but not FBAllocationTracker, see this issue