I'm seeing a rather surprising behaviour, which is that a Swift class is being deallocated (and deinit called) while it is retained (by multiple objects, which are themselves live and not going away). What can cause that?
My assumption, naive as it may be, is that class instances are only deallocated once they are no longer [strongly] referenced; when their retain count at least logically if not literally¹ hits zero. Evidently that's not the case?
The retain count is non-zero by evidence of (a) CFGetRetainCount, (b) Xcode's memory graph debugger, and (c) trivial code inspection because the instance in question is held in an immutable stored property of another class which itself is not deallocated.
I have not yet tried to produce a shareable example of this, because it would likely be difficult (if not impossible), and I'm hoping there's some 'gotcha' that folks can illuminate me on, which will save me the trouble.
¹ I can imagine the compiler skipping the actual decrement of the retain count in memory, if it knows statically that the object is going to be deallocated in any case.
class MyBitmap: NSBitmapImageRep {
final class Data { … } // Just stored properties, init, & deinit.
let data: Data
… // inits etc to initialise `data`.
}
…
let a = MyBitmap(…) // `Data` instance is allocated.
let b = a.retagging(with: …) // Calls `copy` internally.
// lldb says that `b` is a `MyBitmap`.
// `Data` instance has ref-count of 2, as expected.
…
// No more references to `a`. It is deallocated.
// It in turn deallocates the `Data` instance,
// even though `b` is still using it.
// Ref-count of the `Data` instance in its deinit is 2.
I was suspicious that copy might be doing something weird - e.g. just zeroing the data stored property, or blindly copying it without retaining it, but the debugger says this is not the case. And writing an overload which explicitly does all this didn't make any difference (but was definitely called; I checked).
Might this be some oddity of the Objective-C interactions here, NSBitmapImageRep and therefore MyBitmap being NSObject descendants?
It’s definitely possible that NSBitmapImageRep has a custom refcounting implementation that calls [self dealloc] when the refcount hits some magic number. The text system has that, and it even used to be publicly documented.
Even if so, it's not the MyBitmap going away that's the problem per se - it gets retained, released, and eventually deallocated just fine & when expected. It's that it seems to always deallocate its data property, ignoring ref-counting.
Ah, it appears to be a compiler or library bug in how copy / stored properties work in Swift…?
In the default implementation of copy, the strong stored property is just copied by assignment - it does not gain another retain like it should. I guess that doesn't entirely surprise me - evidently the Objective-C runtime does not understand Swift stored properties. Still, that's a pretty darn big caveat, that I've never heard of before.
It's also rather weird, since Objective-C understands the difference between strong and unowned(unsafe) references (copy vs assign attribute, respectively, in property declarations). I assumed Swift stored properties would actually be suitable Objective-C properties, under the hood (for Objective-C classes).
Worse, even if I explicitly implement an override of copy and explicitly assign the stored property, it still doesn't gain the necessary retain. I have to force it using Unmanaged.passRetained(…). I can do that for now, but I worry that it'll introduce a memory leak at some point in the future if this compiler behaviour ever changes. (better than crashing like it does now without the workaround, but still not ideal, obviously)
It's not apparent to me why the retain count is still two when the deinit is called, but I guess that's beside the point.
Ah, you didn’t override -copy? This is not a Swift-specific issue. If a superclass conforms to NSCopying by using NSCopyObject, you must override -copy. This was such a problem for so long that we actually deprecated NSCopyObject to try to encourage developers to safer patterns (like an -initWithCopy: designated initializer).
override func copy() -> Any {
let result = super.copy()
(result as! MyBitmap).data = data
return result
}
Looking at the disassembly, it does call retain - but also release, on the same pointer (because super's copy has already set the copy's data to the source's data). So the net retain count movement is zero. Thus why I have to manually add a retain.
Which is explicable but clearly undesirable - is this the way it's supposed to work? It still seems like fundamentally a bug that super's copy does copy the pointer over but doesn't retain it. It should either not touch it (leaving the pointer as nil) or copy it properly.
Yup, this is the fundamental issue that led to deprecating NSCopyObject(). It simply doesn’t do the right thing. But it was used in some popular classes for 20 to 30 years, so binary compatibility requires it to still be used. See this devforums thread by @eskimo, who links to this blog post by @cocoaphony.
What do I have to do when subclassing NSCell or another class that uses NSCopyObject?
Nothing special. ARC takes care of cases where you had to previously add extra retains explicitly. With ARC, all copy methods should just copy over the instance variables.
Notably, you have to do this via direct assignment, not via calling properties, or else you hit the same double-release issue:
This syntax works regardless of whether your superclass uses NSCopyObject or some other way of copying. I don’t know if there a similar universal spelling for Swift subclasses.
That's exactly it - NSImageRep calls NSCopyObject. Thanks for pinning this down, and providing so much detail!
It seems to confirm my 'hacky' workaround is basically the correct solution, so that's a relief at least. So long as NSImageRep doesn't stop using NSCopyObject, but since it's been a couple of decades it seems like it's in no rush.
:-( The compiler makes an effort to do so, but I guess that only applies to properties that are @objc? I don't remember the details, but that might be a simpler workaround in your case.
Even then, bridged types like Data and String also wouldn't get the same treatment, because their representation is not just a single refcounted pointer that the ObjC runtime can call -copy on. I don't know if I have any good suggestions for that other than overriding -copy to not call super.copy() at all and instead return a completely new object. It Would Be Nice™ if the compiler or ObjC runtime could warn about potentially dangerous uses of NSCopyObject somehow, but I'm not sure what it would do that would be backwards compatible.
