What do you think about this code in Objective-C that iterates through retain count and call release every iteration?
Asked Answered
S

4

8

I'm still trying to understand this piece of code that I found in a project I'm working on where the guy that created it left the company before I could ask.

This is the code:

-(void)releaseMySelf{
    for (int i=myRetainCount; i>1; i--) {
        [self release];
    }
    [self autorelease];
}

As far as I know, in Objective-C memory management model, the first rule is that the object that allocates another object, is also responsible to release it in the future. That's the reason I don't understand the meaning of this code. Is there is any meaning?

Saucy answered 16/12, 2011 at 11:11 Comment(7)
No surprise he left the company; code like that is a sure sign that he was in way over his head, had no understanding of how to write an app and no opportunity to learn the proper ways. If I found that in a project (and I have -- used run a consulting services company that did "clean up" on troubled projects), I would immediately throw a red flag and demand that every line of code written by that person be reviewed. (It also looks like he is maintaining a retain count separate from the object's actual retain count... 2 red flags on that play.)Myriapod
I think "I want to know who wrote that so I can be sure to never work with him"Ingurgitate
Oy vey. I just threw up in my mouth.Ainslee
Where does myRetainCount come from? Where is it defined? Where else is it used? Without that, it's tough to judge whether the code might not be OK after all. Well, maybe not OK, but at least not buggy, just bad style.Kolk
@Kolk Um, no. No matter what myRetainCount does, this is not OK.Mystify
@Kolk If he were implementing his own reference counting system on top of the system's RC, then that could be valid code, though it would also require some kind of a solution like zeroing-weak-on-dealloc to make it correct. At best, a bad re-invention of wheels.Myriapod
The funniest code I've ever read. LOL :))Sea
H
17

The author is trying to work around not understand memory management. He assumes that an object has a retain count that is increased by each retain and so tries to decrease it by calling that number of releases. Probably he has not implemented the "is also responsible to release it in the future." part of your understanding.

However see many answers here e.g. here and here and here.

Read Apple's memory management concepts.

The first link includes a quote from Apple

The retainCount method does not account for any pending autorelease messages sent to the receiver.

Important: This method is typically of no value in debugging memory management issues. Because any number of framework objects may have retained an object in order to hold references to it, while at the same time autorelease pools may be holding any number of deferred releases on an object, it is very unlikely that you can get useful information from this method. To understand the fundamental rules of memory management that you must abide by, read “Memory Management Rules”. To diagnose memory management problems, use a suitable tool: The LLVM/Clang Static analyzer can typically find memory management problems even before you run your program. The Object Alloc instrument in the Instruments application (see Instruments User Guide) can track object allocation and destruction. Shark (see Shark User Guide) also profiles memory allocations (amongst numerous other aspects of your program).

Heel answered 16/12, 2011 at 11:23 Comment(1)
On a side note, I recommend you removing this piece of code and running your code through static analyzer. It should show where objects should've been released.Landel
K
7

Since all answers seem to misread myRetainCount as [self retainCount], let me offer a reason why this code could have been written: It could be that this code is somehow spawning threads or otherwise having clients register with it, and that myRetainCount is effectively the number of those clients, kept separately from the actual OS retain count. However, each of the clients might get its own ObjC-style retain as well.

So this function might be called in a case where a request is aborted, and could just dispose of all the clients at once, and afterwards perform all the releases. It's not a good design, but if that's how the code works, (and you didn't leave out an int myRetainCount = [self retainCount], or overrides of retain/release) at least it's not necessarily buggy.

It is, however, very likely a bad distribution of responsibilities or a kludgey and hackneyed attempt at avoiding retain circles without really improving anything.

Kolk answered 16/12, 2011 at 18:7 Comment(2)
However it is still not the correct way to implement - it is still the author not getting memory management correct - retain/release etc works with threadsHeel
Threads were just meant as an example of clients that one would typically register/unregister, e.g. for thread pools or "cancel all" behaviour.Kolk
P
3

This is a dirty hack to force a memory release: if the rest of your program is written correctly, you never need to do anything like this. Normally, your retains and releases are in balance, so you never need to look at the retain count. What this piece of code says is "I don't know who retained me and forgot to release, I just want my memory to get released; I don't care that the others references would be dangling from now on". This is not going to compile with ARC (oddly enough, switching to ARC may just fix the error the author was trying to work around).

Pasteurization answered 16/12, 2011 at 11:25 Comment(0)
F
2

The meaning of the code is to force the object to deallocate right now, no matter what the future consequences may be. (And there will be consequences!)

The code is fatally flawed because it doesn't account for the fact that someone else actually "owns" that object. In other words, something "alloced" that object, and any number of other things may have "retained" that object (maybe a data structure like NSArray, maybe an autorelease pool, maybe some code on the stackframe that just does a "retain"); all those things share ownership in this object. If the object commits suicide (which is what releaseMySelf does), these "owners" suddenly point to bad memory, and this will lead to unexpected behavior.

Hopefully code written like this will just crash. Perhaps the original author avoided these crashes by leaking memory elsewhere.

Formality answered 16/12, 2011 at 17:36 Comment(1)
Thanks a lot mates! That's what I think, this is a horrible code.Saucy

© 2022 - 2024 — McMap. All rights reserved.