[Openmcl-devel] Cocoa memory management and containers

[Ron Garret]

Sorry for not responding sooner, I’m on a cruise ship and our internet connection died overnight.

I believe everything you say here is correct.  But just for the record, I’m pretty sure that the following (specifically, my failure to #/release the bitmap object) accounts for the original leak:

? (setf bm (#/alloc ns:ns-bitmap-image-rep))
#<NS-BITMAP-IMAGE-REP [uninitialized] (#x6080000B8600)>
? (#/retainCount bm)
1
? (ns:with-ns-rect (r 0 0 10 10)
    (with-focused-ns-image img
      (#/initWithFocusedViewRect: bm r)))
;Compiler warnings :
;   In an anonymous lambda form at position 96: Undeclared free variable BM
;   In an anonymous lambda form at position 58: Undeclared free variable IMG (2 references)
#<NS-BITMAP-IMAGE-REP NSBitmapImageRep 0x6080000b8600 Size={10, 10} ColorSpace=sRGB IEC61966-2.1 colorspace BPS=8 BPP=32 Pixels=20x20 Alpha=YES Planar=NO Format=0 CurrentBacking=<CGImageRef: 0x6000003a41a0> (#x6080000B8600)>
? (#/retainCount bm)
1
? (#/addRepresentation: img bm)
NIL
? (#/retainCount bm)
2
? (#/release img)
NIL
? (#/retainCount bm)
1
? 


On Feb 7, 2020, at 8:09 AM, Paul Krueger <plkrueger at comcast.net> wrote:

> OK, much of what I wrote in my previous post is bogus. My excuse is that it’s been a while since I’ve thought about much of this. The (#/alloc …) will return an object with a retaincount of 1, so you were correct that this will need to be released before it will be deallocated. You can do that with a #/release after you have added it to the img or you can do an #/autorelease anytime you want.
> 
> I will reiterate one other issue with your code that I see. That is that init functions (in general) need not return the same object that was passed into the init. If that had been an ns:ns-string object that you were initing with #/initWithString:, then the object returned from the call would have been different and the variable passed into the init call would then have pointed to a deallocated (bogus) object (because that particular init releases the object that is the target of the init). You can get away with the way your code is structured only when the init function you are using returns the same object. You can find online discussions about why you have to be careful about splitting alloc and init in the way that you do.
> 
> I suppose the other thing you need to be aware of is that “convenience constructors” (see Apple’s docs) will generally return an object that has already been autoreleased so that you do not need to worry about doing that yourself.
> 
>> On Feb 7, 2020, at 9:01 AM, Paul Krueger <plkrueger at comcast.net> wrote:
>> 
>> Be careful. What you’re saying here doesn’t sound exactly like what I was suggesting. Creating Bitmap in the way you do does not increment its refcount. Unless you #/retain it somewhere that you don’t show in your example, you should be able to do the #/addRepresentation: as you did and then forget about it. The img object will retain it, as you suggested and when img is #/released then if and only if the img refcount goes to zero the bitmap object will be released. Then if and only if the bitmap object refcount goes to zero will it be deallocated. 
>> 
>> What I was trying to suggest is that maybe when the img object was released that its refcount wasn’t at zero and therefore did not release the bitmap object that it contained. 
>> 
>> It is, of course, also true that if the img refcount DID go to zero and it released the bitmap but ITS refcount wasn’t zero then the bitmap would not be deallocated. That sounds like what you are suggesting. But if that wasn’t what happened and you did an extra release on the bitmap when you hadn’t done a retain anywhere, that could prematurely deallocate it while something else still held a link to it. That could cause a crash later on when that link was referenced or something tried to release the bitmap after it was deallocated. I won’t get into the subject of autorealease pools here, but they could be playing a role too. That’s especially tricky when you’re trying to debug interactively because there is an autorelease pool associated with the listener that may keep things around even after your code seemed to be done with them. Aren’t refcounts fun?
>> 
>>> On Feb 6, 2020, at 7:54 PM, Ron Garret <ron at flownet.com> wrote:
>>> 
>>> Aha!  That’s the answer.
>>> 
>>> When you add an object to a container, the container add a refcount.  But then you need to release the object to turn over ownership to the container.
>>> 
>>> Kind of obvious in retrospect.
>>> 
>>> Thanks!
>>> 
>>> rg
>>> 
>>> On Feb 6, 2020, at 8:29 AM, Paul Krueger <plkrueger at comcast.net> wrote:
>>> 
>>>> OK, here’s another thought. Make sure that when img is released that its ref count actually goes to zero. If not, then it won’t be dealloc’ed and therefore won’t release bitmap. Maybe something else is holding on to a reference to it.
>>>> 
>>>>> On Feb 5, 2020, at 8:00 PM, Ron Garret <ron at flownet.com> wrote:
>>>>> 
>>>>> This is not a CCL question, it’s a Cocoa memory management question, but since i’m not plugged in to the Cocoa development community and I actually am developing in CCL I thought I’d ask here.
>>>>> 
>>>>> I want to extract and use bitmap representations of ns-images.  I wrote the following code:
>>>>> 
>>>>> (defun image->bitmap (img)
>>>>> ;; Check to see if the image already has a bitmap
>>>>> (or
>>>>> (loop
>>>>> with e = (#/objectEnumerator (#/representations img))
>>>>> as rep = (#/nextObject e)
>>>>> until (%null-ptr-p rep)
>>>>> if (typep rep ns:ns-bitmap-image-rep) return rep)
>>>>> ;; Otherwise create one and add it to the image
>>>>> (bb
>>>>> :mv (w h) (size img)
>>>>> bitmap (#/alloc ns:ns-bitmap-image-rep) ;; Memory leak
>>>>> (ns:with-ns-rect (r 0 0 w h)
>>>>>  (with-focused-ns-image img
>>>>>    (#/initWithFocusedViewRect: bitmap r)))
>>>>> (#/addRepresentation: img bitmap)
>>>>> bitmap)))
>>>>> 
>>>>> The BB macro is a general-purpose binding construct (it stands for binding-block).  :MV is multiple-value-bind.  Those details don’t really matter.  The only thing that matters is the logic flow: I first check to see if the image already has a bitmap and if so I return that, otherwise I create a new one.
>>>>> 
>>>>> The problem is that this leaks memory because (AFAICT) the bitmap that I create is NOT released when the image is released.
>>>>> 
>>>>> AFAICT this is a general problem with ObjC containers.  Putting an object inside a container does NOT increment its reference count, and likewise, removing it does not decrement the reference count.
>>>>> 
>>>>> This seems to me to undermine the whole point of reference counts, and makes it nearly impossible to write a non-leaky version of IMAGE->BITMAP.  The only way I can think of to do it is to create a superclass of ns-image that somehow does the Right Thing when it is released.  But I have a hard time believing that this is really the right answer.  Surely Cocoa memory management is not really this brain-dmanaged, and there is something that I have overlooked?
>>>>> 
>>>>> Advice from anyone more wise in the ways of ObjC would be most welcome.
>>>>> 
>>>>> Thanks,
>>>>> rg
>>>>> 
>>>>> _______________________________________________
>>>>> Openmcl-devel mailing list
>>>>> Openmcl-devel at clozure.com
>>>>> https://lists.clozure.com/mailman/listinfo/openmcl-devel
>>>> 
>>> 
>> 
>> _______________________________________________
>> Openmcl-devel mailing list
>> Openmcl-devel at clozure.com
>> https://lists.clozure.com/mailman/listinfo/openmcl-devel
>