Matt Rajca

TL;DR: don’t assume NSDocuments will be deallocated on the main thread on macOS Sierra and do any clean-up work in NSDocument.close.

In testing Robotary on macOS Sierra, I noticed that, every once in a while, the deinit method of one of the objects owned by our NSDocument subclass was being invoked on a background thread. More specifically, the deinit method looks something like this:

deinit {
	assert(Thread.isMainThread)
	close()
	/* other clean-up work */
}

Since the close method of this class is not thread-safe, it has to be invoked on the main thread. However, we were fairly certain that NSDocuments would never be deallocated on a background thread, so we simply put an assertion in there. Starting with macOS Sierra, we found this assertion started triggering exceptions every once in a while.

After using the Allocations instrument in Instruments and tracing the retains and releases of the NSDocument subclass, it was obvious what was happening. Internally, Cocoa was scheduling some iCloud-related work (namely an NSBlockOperation) on a background queue right around the time documents were being closed, and this operation was retaining the NSDocument subclass. Most of the time, the operation would finish before the document’s UI got torn down, so the deinit method would be called on the main thread. Every once in a while, however, this operation would run a little late and end up being the last to hold on to our NSDocument subclass. This would cause the document’s deinit method to get invoked on a background thread.

I don’t think this is an AppKit bug per-se, but it can lead to subtle issues in your code. I also haven’t seen any mentions of this edge case in the AppKit Release Notes. If you have to perform clean-up work in the deinit method of your NSDocument subclasses (or any of the objects they own), it’s better to override NSDocument.close and do it there:

override func close() {
	assert(Thread.isMainThread)
	super.close()
	closeOtherResources()
}