0

In the following scenario I get a crash

    if (self.videoEngine != nil)
{
    [self.videoEngine.player.view removeFromSuperview];

    [videoEngine release];
    self.videoEngine = nil;
}

The videoEngine object is (nonatomic, retain), and it is synthesized using videoEngine = _videoEngine. If I remove the self.videoEngine = nil line the code works properly. Is this correct behaviour, and why does the nil line cause a crash? Would the self.videoEngine = nil still cause an issue within the viewDidUnload function?

Elliott D'Alvarez
  • 1,217
  • 16
  • 30

2 Answers2

5

When you call "self.videoEngine = nil;" it calls its setter method and in the setter method by default it releases the object and then it sets it to the value provided by you, so in this case you are releasing your object once and then setter method is trying to release it again that is causing crash, now if you remove the "[videoEngine release];" that would be fine and there will be no memory leak.

Hope it is clear now.

Nilesh
  • 1,493
  • 18
  • 29
  • Thanks very much, so using self.videoEngine = nil is better than releasing? Or do I need to release _videoEngine in dealloc still? – Elliott D'Alvarez Jul 23 '12 at 12:43
  • 1
    It is preferred that you should set nil your object after releasing your object, and if you use self.object = nil then it will automatically released and seted to nil. – Nilesh Jul 23 '12 at 14:48
  • And it is good practice to check object before you release it like : if(self.object){ self.object = nil; } – Nilesh Jul 23 '12 at 14:54
  • @Nilesh: No it is not. That is completely unnecessary and makes no difference. – newacct Jul 23 '12 at 18:08
  • Retained objects should be released directly: [_videoEngine release]; it should not be released through the setter as: self._videEngine = nil; (Wrong). But if you have an assigned property then the setter is set to self.assignedProperty = nil; to prevent dangling pointers, Note this applies only to (assigned) properties, which should not be released, but may confuse some people because they have to look in the header file to see the difference of the properties. – Sverrisson Jul 23 '12 at 18:13
0

You should only release _videoEngine because that is the the synthesized name. videEngine is only the name of the setter and getter, but the value is stored in the syntheseized name. So your code should be:

    if (self.videoEngine != nil)
{
    [self.videoEngine.player.view removeFromSuperview];

    [_videoEngine release];
    self.videoEngine = nil;   // Unnecessary

}

But you don´t need to call self.videEngine = nil after releasing the _videEngine because the setter will always return nil.

It is not considered a proper method of releasing by calling the setter method with nil, although it works, like is done with the line: self.videoEngine = nil; // Unnecessary. The proper way of releasing is only [_videoEngine release];

Sverrisson
  • 17,970
  • 5
  • 66
  • 62
  • Oh really? So now I'm confused, I should be releasing and not using the nil method? Or do they effectively do the same thing? – Elliott D'Alvarez Jul 23 '12 at 13:22
  • You should release the synthesized variable [_videoEngine release]; after that you don´t need to do anything else. You should not use self.videoEngine = nil; because you are sending a nil to the setter and it may require some processing time. – Sverrisson Jul 23 '12 at 13:27
  • 1
    You are confused because you mixed videoEngine and _videoEngine – Sverrisson Jul 23 '12 at 13:30
  • Oh yeah, thanks for pointing out that one is wrong! It was fine on all the others but must have missed it :) – Elliott D'Alvarez Jul 23 '12 at 13:40
  • `[_videoEngine release];` should be followed by `_videoEngine = nil;` (or alternately `self.videoEngine = nil;` does both) unless this is in `dealloc`. Otherwise you will have a dangling pointer. – newacct Jul 23 '12 at 18:07