Uploaded image for project: 'Apache Cordova'
  1. Apache Cordova
  2. CB-401

Crash on iOS when taking multiple pictures

Details

    • Bug
    • Status: Closed
    • Critical
    • Resolution: Duplicate
    • 1.4.0
    • None
    • cordova-ios
    • None
    • 1.4.1, 1.5.0 on iOS
      Crash on iPad (third generation)

    Description

      We have an app that allows the user to take four pictures. It had been working fine but we started seeing crashes when we tested on a new iPad (third generation).

      We have eliminated the crash in our application by making the following change to the PhongeGap/Cordova code. I tested with PhoneGap 1.4.1. I'm currently upgrading to Cordova 1.5.0 but a review of Camera.m/CDVCamera.m shows only the name changes.

      Line numbers below correspond to CDVCamera.m.

      In CDVCamera.m, we found the following code around line 67:

      if (self.pickerController == nil) 
      {
          self.pickerController = [[[CameraPicker alloc] init] autorelease];
      }
      

      We removed the autorelease message:

      if (self.pickerController == nil) 
      {
          self.pickerController = [[CameraPicker alloc] init];
      }
      

      Our reason for doing this is that we found that self.pickerController appears to be released in all the right places:

      • at line 220 in imagePickerController:didFinishPickingMediaWithInfo:
      		self.pickerController = nil;
      
      • at line 251 in imagePickerControllerDidCancel:
      		self.pickerController = nil;
      
      • at line 399 in dealloc:
      		self.pickerController = nil;
      

      This last one may not always be necessary but should be okay since the previous releases set the instance variable to nil. And, of course, since pickerController is a property, setting it to nil serves to release it.

      I think both autoreleasing and explicitly releasing an object is not following the memory management rules. Apple says:

      My thought (confirmed with an instructor at Big Nerd Ranch) is that "or" is the key word in this statement and removing the extra autorelease could certainly prevent zombie crashes that only occur in certain circumstances.

      This issue seems like a duplicate of CB-264 but since that one is resolved as "won't fix," I have submitted this new issue containing a possible solution.

      Attachments

        Issue Links

          Activity

            This is incorrect. Autoreleasing it there is correct. Let me explain.

            self.pickerController = [[[CameraPicker alloc] init] autorelease];

            After this, the retain count is 1. If you removed the autorelease, the retain count is 2, which is incorrect, and the Static Analyzer will back me up on this. The property is a retain property.

            You would be correct if we are dealing with naked ivars, but these are synthesized properties that are retained - @synthesize is just syntactic sugar, this is how it works internally: http://stackoverflow.com/questions/5350563/what-equivalent-code-is-synthesized-for-a-declared-property

            Also, if it's nil, setting it to nil again will not send the release message on it again, so it's safe (again, a retained synthesized property thing, see above link).

            The actual problem has been found in CB-391. Closing as a dupe of CB-391.

            shazron Shazron Abdullah added a comment - This is incorrect. Autoreleasing it there is correct. Let me explain. self.pickerController = [[ [CameraPicker alloc] init] autorelease]; After this, the retain count is 1. If you removed the autorelease, the retain count is 2, which is incorrect, and the Static Analyzer will back me up on this. The property is a retain property. You would be correct if we are dealing with naked ivars, but these are synthesized properties that are retained - @synthesize is just syntactic sugar, this is how it works internally: http://stackoverflow.com/questions/5350563/what-equivalent-code-is-synthesized-for-a-declared-property Also, if it's nil, setting it to nil again will not send the release message on it again, so it's safe (again, a retained synthesized property thing, see above link). The actual problem has been found in CB-391 . Closing as a dupe of CB-391 .
            marcrhodes Marc Rhodes added a comment -

            You are, of course, correct. As you noted, I managed to overlook that this was an assignment to a property not an ivar. I appreciate your kind explanation and apologize for the unnecessary report based on a mistake in my analysis.

            marcrhodes Marc Rhodes added a comment - You are, of course, correct. As you noted, I managed to overlook that this was an assignment to a property not an ivar. I appreciate your kind explanation and apologize for the unnecessary report based on a mistake in my analysis.

            People

              shazron Shazron Abdullah
              marcrhodes Marc Rhodes
              Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: