Crash when assigning NSImage to `@objc dynamic var` property

I just created FB19465448. Any help is very much appreciated.

The code is called on the main thread. According to the crash report, thread 0 makes that assignment,

Looking at the crash log, the abort originates here:

4   libobjc.A.dylib               	0x00000001818d7fc0 _objc_fatal(char const*, ...) + 44 (objc-errors.mm:232)
5   libobjc.A.dylib               	0x00000001818a6d78 weak_register_no_lock + 396 (objc-weak.mm:423)
6   libobjc.A.dylib               	0x00000001818a6b40 objc_storeWeak + 472 (NSObject.mm:408)

As it happen, you don't have to guess at why weak_register_no_lock failed, as the project is opensource. Here is the line that's crashing:

_objc_fatal("Cannot form weak reference to instance (%p) of "
			"class %s. It is possible that this object was "
			"over-released, or is in the process of deallocation.",
			(void*)referent, object_getClassName((id)referent));

That leads to here:

and at the same time thread 16 is calling [NSImageView asynchronousPreparation:prepareResultUsingParameters:].

Unfortunately, I wouldn't necessarily assume this call is directly involved. From the large context in the logs you sent to DTS, it looks like you have some kind of processing pipeline at work, so it's possible that's an unrelated thread. The best suggestion I have here is to start with the advice in "Investigating memory access crashes".

__
Kevin Elliott
DTS Engineer, CoreOS/Hardware

Thanks. So it sounds like the NSImage is getting over-released or being incorrectly deallocated, although it's unclear to me what exactly is trying to form a weak reference, since my code (as shown above) has a strong reference. Unfortunately the higher up function calls are made by AppKit, which is not open source, so I cannot look it up.

As far as I understand, the link you posted helps investigating memory issues in Xcode, but since the crash reports are downloaded by Xcode from other users and I cannot reproduce it myself...

The Xcode statistics seem to show that it only happens with macOS 15.3 or newer. I don't know if it's because there's not enough space to show older releases, or if it's really a clue that it's a change introduced with macOS 15.3 that causes this issue.

From my perspective it would make sense that it's a new issue with macOS 15.3, because I haven't changed the code that generates or assigns that image in a very long time, and this issue didn't happen for a previous version of my app that was live on the App Store for 6 months (starting from about one year ago) and had barely any crashes (all unrelated to this one).

Perhaps it helps to share how the image is created. The result of the method below is directly assigned to myEntity.image in the code I posted originally. Could any of the used APIs have a bug that causes the image to be released incorrectly?

private func image(url: URL?) -> NSImage {
    if let url = url {
        let imageURL: URL
        if let volumeURL = (try? url.resourceValues(forKeys: [.volumeURLKey]))?.volume, volumeURL.path != "/" {
            imageURL = volumeURL
        } else {
            imageURL = url
        }
        return (try? imageURL.resourceValues(forKeys: [.effectiveIconKey]))?.effectiveIcon as? NSImage ?? NSImage(named: NSImage.folderName)!
    } else {
        return NSImage(systemSymbolName: "questionmark.diamond.fill", accessibilityDescription: nil)!.withSymbolConfiguration(.init(paletteColors: [NSColor(white: 0.95, alpha: 1), .systemPurple]))!
    }
}

Thanks. So it sounds like the NSImage is getting over-released or being incorrectly deallocated, although it's unclear to me what exactly is trying to form a weak reference, since my code (as shown above) has a strong reference. Unfortunately, the higher up function calls are made by AppKit, which is not open source, so I cannot look it up.

Seeing that code wouldn't actually help. The thing that makes over-release issues (and most other memory crashes ) hard to debug is that the crash log you’re looking at is NOT why your app crashed. At some earlier point in your app, "something" happened in your app that caused an "extra" release that would not normally occur.

In other words, what the crash log shows is a "victim" of an underlying issue, not its direct cause.

As far as I understand, the link you posted helps investigating memory issues in Xcode, but since the crash reports are downloaded by Xcode from other users and I cannot reproduce it myself...

I should have posted more direct links, but most of the memory-related debug libraries Xcode includes are specifically designed with this issue in mind. That is, they're designed to try and find hard-to-reproduce memory issues, typically by "forcing" your app to crash under circumstances when it normally would not.

As the simplest to explain example, under normal circumstances, accessing a recently freed object often "works" fine. As long as the object hasn't been overwritten, the data will be exactly the same as it was before it was freed, so everything "works". However, what the Zombies instrument actually does is modify the "dealloc" method so that freeing an object doesn't ACTUALLY free that object, but instead replaces it with an intentionally invalid object ("the zombie"). That means that all accesses to that previously freed object will not crash and, conveniently, the zombie object can also track the object’s previous retain/release history, so you can see exactly where things went wrong.

Our lower-level tools like ASan, TSan, UBSan, and the Main Thread Checker are all designed with that same mindset— find a bug by forcing crashes to occur that would otherwise require very specific timing or circumstances.

I can't guarantee that any of these tools will find your bug, but issues like this can be extremely difficult to investigate, so any "easy" testing option is worth pursuing.

The Xcode statistics seem to show that it only happens with macOS 15.3 or newer. I don't know if it's because there's not enough space to show older releases, or if it's really a clue that it's a change introduced with macOS 15.3 that causes this issue.

The word "cause" is tricky here. As I talked about above, issues like this are often heavily dependent on the timing of activity inside your app. Because of that, it's not at all unusual for an issue to suddenly appear or increase with a particular system release or app change. However, that doesn't mean that a system or app update caused the issue or that the solution is to "undo" that change; it may just mean that those timing changes exposed an issue that was previously hidden.

From my perspective, it would make sense that it's a new issue with macOS 15.3.

There isn't really any way for me to know for certain that this isn't a bug in the system; however, I can say that this doesn't match any crash signature we're actively tracking or investigating. If it is a system bug, it's not a very common one.

Perhaps it helps to share how the image is created. The result of the method below is directly assigned to myEntity.image in the code I posted originally. Could any of the used APIs have a bug that causes the image to be released incorrectly?

What thread is myEntity.image accessed on? If it's assigned on one thread and accessed on another, then that can cause exactly the kind of crash you're seeing.

__
Kevin Elliott
DTS Engineer, CoreOS/Hardware

I don't understand. Aren't all these tools you mention (Zombies instrument, ASan, Main Thread Checker etc.) meant to be run in Xcode or Instruments? Or are you saying I should enable some compiler flag in my production build?

The thing that makes over-release issues (and most other memory crashes ) hard to debug is that the crash log you’re looking at is NOT why your app crashed.

I'm not sure I understand. Is it correct that the line in the crash log

myEntity.image = newImage

is trying to retain an image that was over-released or is being deallocated, but the thing that over-released it is possibly somewhere else?

What thread is myEntity.image accessed on?

I mentioned in my original post that it's called on the main thread, and I just checked again and can confirm it. Just to be sure, I'll put a

if !Thread.isMainThread {
    preconditionFailure()
}

in the next app update, so there won't be a shadow of a doubt anymore. Though it will take a couple weeks before the update is published and the first crash reports will start coming in again.

This brings me to:

Seeing that code wouldn't actually help.

Even not if we assume that it all happens on the main thread? Could there be something else wrong with my code, or could this be a bug in how these images are created by Foundation / AppKit?

I don't understand. Aren't all these tools you mention (Zombies instrument, ASan, Main Thread Checker etc.) meant to be run in Xcode or Instruments? Or are you saying I should enable some compiler flag in my production build?

They're intended to run in your development build. Let me jump back to what you're saying here:

since the crash reports are downloaded by Xcode from other users and I cannot reproduce it myself...

What you're saying here is "testing in Xcode won't help because I can't reproduce the issue". The problem with that statement is that it misunderstands how the tools above actually work. They don't just provide better diagnostic data about a crash that would otherwise occur, they actually CREATE crashes that would otherwise NOT occur. Jumping back to my description here:

However, what the Zombies instrument actually does is modify the "dealloc" method so that freeing an object doesn't ACTUALLY free that object, but instead replaces it with an intentionally invalid object ("the zombie"). That means that all accesses to that previously freed object will not crash...

Under normal circumstances, accessing the memory of a previously deallocated object doesn't "inherently" crash. It might crash or it might not, depending entirely on what exactly has happened to that memory since it deallocated. With Zombies active, that exact same usage will crash 100% of the time. That pattern is true of most of those tools— they change your app’s runtime behavior so that bugs that are normally hidden by the internals of your app crash immediately.

I'm not sure I understand. Is it correct that the line in the crash log

myEntity.image = newImage

The issue here is that when you assign a new value to "myEntity.image", you’re implicitly releasing its old value. That could be a problem, depending on what "else" is happening with that image.

However, I want to be careful about pointing toward any specific cause. That's the problem with crashes like this— they AREN'T simple and they generally DON'T have any specific cause. What makes these crashes random/rare is that they typically require multiple things to have happened in a particular pattern before the crash will occur, which means focusing too closely on any specific cause isn't all that helpful.

Shifting to here:

Is trying to retain an image that was over-released or is being deallocated, but the thing that over-released it is possibly somewhere else?

First off, as background I'd recommend reading through "Objective-C Memory Management for Swift Programmers". With that context the quick summary is that our memory management works by having an object start with a retain count of "1", incrementing/decrementing that retain count as the object is manipulated, then freeing it when the retain count goes to zero. So, over an object’s lifetime its retain count might look something like this:

Object Created-> 1 -> 2-> 3-> 2-> 3-> 2-> 1-> 0-> Object Destroyed.

Your app is crashing because it's accessing the object after it was destroyed (that's what "over release" means). That also means that wherever you crash probably isn't directly responsible, as the "extra" release happened at some earlier point.

Even not if we assume that it all happens on the main thread?

Yes. More specifically:

• As I talked about above, whatever is going on doesn't involve some simple code flow, otherwise it would happen all the time, so you wouldn't be asking for help.

• Similarly, the code that's directly "in" the crash log is unlikely to be the direct cause of the crash. It may be related to it, but it probably isn't the direct source.

Could there be something else wrong with my code, or could this be a bug in how these images are created by Foundation / AppKit?

That's a question that can't be answered until you find the problem.

__
Kevin Elliott
DTS Engineer, CoreOS/Hardware

they actually CREATE crashes

Thanks, that's what I should have known but didn't fully realize until now. I just tried enabling the Address and Thread Sanitizer (the Main Thread Checker is already enabled), but couldn't reproduce the crash. I'll keep them enabled in the hope that they will create a crash at some point.

whatever is going on doesn't involve some simple code flow

I could understand that this crash could be caused by multiple threads releasing the image at the same time, but if only the main thread is accessing them, I cannot imagine how they could be over-released. I'm not doing any manual memory management with them or their parent object. If you have any example of how over-releasing could happen without thread contention and manual memory management, I'd be eager to hear it. Otherwise I'll let you know as soon as I'm able to reproduce the crash.

I could understand that this crash could be caused by multiple threads releasing the image at the same time, but if only the main thread is accessing them, I cannot imagine how they could be over-released. I'm not doing any manual memory management with them or their parent object. If you have any example of how over-releasing could happen without thread contention and manual memory management, I'd be eager to hear it. Otherwise, I'll let you know as soon as I'm able to reproduce the crash.

So, I took another look at your crash log and our code and had a bit more information to share. So, let me start here:

7   AppKit                        	0x000000018667d860 -[_NSAsynchronousPreparation initWithDelegate:parameters:] + 104 (NSAsynchronousPreparation.m:97)

After a second pass through our code, I think the object that's actually at issue here isn't the image, it's the NSImageView. A few things to look at due to that:

• As a general comment, KVOs’ reputation is extremely... mixed. My own opinion is deeply skewed by my experience with it (people don't come to DTS because of how well their app is working...), but my own view is that it is an API that looks very nice in simple demos but that the complexity and bug risk it introduces aren't worth the perceived simplicity it provides.

• In terms of reproducing the issue, look for any kind of interaction that would destroy the image view, especially if it's happening at the same time the image itself is changing.

• When any image view is being destroyed, make sure you're setting the target image to null and you've torn down any KVO "infrastructure" you've created that's tied to that object.

That leads to here:

I'm not doing any manual memory management with them or their parent object. If you have any example of how over-releasing could happen without thread contention and manual memory management, I'd be eager to hear it.

That's generally a reasonable statement for the "core" of Objective-C’s memory management model, but unfortunately, KVO is VERY far away from that model.

One comment on the testing front:

I just tried enabling the Address and Thread Sanitizer (the Main Thread Checker is already enabled), but couldn't reproduce the crash.

I can't promise it will work, but I would also try Zombies. One of the reasons it's still around is that it's directly "bound" into Objective-C’s memory system in a way that none of our other tools are, which means it can still catch things the others don't.

__
Kevin Elliott
DTS Engineer, CoreOS/Hardware

When any image view is being destroyed, make sure you're setting the target image to null and you've torn down any KVO "infrastructure" you've created that's tied to that object

The NSImageView is in fact in a NSTableView, and I call NSImageView.bind(_:to:withKeyPath:options:) in the callback passed to NSObject.observe(_:options:changeHandler). The observer is added when the table cell view's objectValue is set, then removed again when it is set to nil. I'll add a call to NSImageView.unbind(.value) and see if that solves the crash.

class MyCellView: NSTableCellView {
    
    private var observer: NSKeyValueObservation?
    
    override var objectValue: Any? {
        didSet {
            if let objectValue = objectValue as? MyObject {
                observer = objectValue.observe(\.property) { [weak self] _, _ in
                    for view in subviews {
                        view.removeFromSuperview()
                    }
                    let imageView = NSImageView(image: nil)
                    imageView.bind(.value, to: objectValue, withKeyPath: keyPath)
                    addSubview(imageView)
                }
            } else {
                observer = nil
            }
        }
    }
    
}

That's generally a reasonable statement for the "core" of Objective-C’s memory management model

Not sure if it's relevant, but I'm using Swift, although I suspect KVO is at the Objective C level.

I would also try Zombies

Sorry, forgot to add that I also enabled that one, but no crash for now. I'll keep that enabled too.

Thanks for your precious input and I'll update you when I find out more.

The NSImageView is in fact in an NSTableView, and I call NSImageView.bind(:to:withKeyPath:options:) in the callback passed to NSObject.observe(:options:changeHandler). The observer is added when the table cell view's objectValue is set, then removed again when it is set to nil. I'll add a call to NSImageView.unbind(.value) and see if that solves the crash.

Your code has me a bit confused. How do you modify the contents of the NSImageView? Are you:

1. Modifying the property of MyObject? (going through the binding)

2. Modifying objectValue? (directly modifying the view cell)

It feels like you're somewhat awkwardly set up to do both, which seems like an unnecessary complication. In the first case, there's no reason to remove any views, as you're simply doing a one-timing binding to a specific NSImageView. In the second case, you could drop binding entirely and simply directly assign the image whenever it changes.

My concern here is that mixing both puts you in an odd situation, since going through #2 will remove the existing NSImageView from the view hierarchy while still leaving it connected to the image of the old value.

Not sure if it's relevant, but I'm using Swift, although I suspect KVO is at the Objective-C level.

KVO is ENTIRELY Objective-C and, in fact, heavily relies on very obscure details of Objective-C’s architecture and runtime. Swift is only a very thin layer on the underlying implementation.

I don't know how central this is to your app’s architecture, but unless you're getting some MASSIVE advantage from this, my actual advice would be to rework what you're doing so you can remove both KVO and bindings. Frankly, it's very easy to end up spending more time trying to figure out "what caused the problem" than would to write a simpler solution that can't fail*.

*The classic argument made for KVO and bindings is that they "get rid of boilerplate". The counterargument to that is that replacing boilerplate you can understand and debug with "magic" that you can't understand or debug isn't necessarily a good trade-off.

__
Kevin Elliott
DTS Engineer, CoreOS/Hardware

Your code has me a bit confused.

I should have gone more into details. The property in objectValue.observe(\.property) is actually an array of objects that represent a file URL, and in the observer callback I iterate through that array, create an image view for each element, and bind the image to a property of the respective element. The table view cell effectively displays a dynamic set of URL icons. I modify MyObject.property (I should have called it MyObject.entities) whenever an element is added or removed, and I modify MyEntity.image whenever the image changes. objectValue is set by the table view. I'll think if I can avoid using KVO, but this means I'll have to reload the table view cells manually in quite some places.