macOS Is NSDocument's Saving Message Flow Threaded?

[JonDann]

Hi Guys,

I've overridden -canCloseDocumentWithDelegate:shouldCloseSelector:contextInfo: to display a custom NSAlert if any of the text files that my NSDocument subclass manages are not saved.

The problem is, if the user tries to close the window and very quickly presses return to select the default response of "Save All Files" the app crashes when [self close] is called. If they wait a second or two then no crash happens. The "saveAllFiles" method writes the text files to the disk and then saves the NSDocument model, which holds all the file references.

From my logs I can see that the [self close] causes the document to be dealloc'd before -dataOfType:error: can be called. With NSZombie enabled I get the following log output

2008-02-16 16:57:25.803 Scribbler[697:10b] *** -[ESDocument _saveDocumentWithDelegate:didSaveSelector:contextInfo:]: message sent to deallocated instance 0xea625c0

Where the instance 0xea625c0 is the ESDocument that is being saved, then closed.

Could this be a threading issue with the way NSDocument saves itself, even though I haven't put any threaded code in my app at all? I ask this as the time interval between hitting command-W and return seems to matter.

Here's the relevant code from my NSDocument:

- (IBAction)saveAllFiles:(id)sender;
{
	NSLog(@"%p %s",self,__func__);
	for (ESNode *node in self.projectFiles) {
		if (node.isEdited) {
			NSError *writeError;
			if (![node writeBodyToFileError:&writeError]) {
				NSLog(@"%p %s %@",self,__func__,writeError);
				[self presentError:writeError];
			}
		}
	}
	
	[self saveDocument:nil];
}

- (void)unsavedFilesAlertDidEnd:(NSAlert *)alert returnCode:(NSUInteger)returnCode contextInfo:(void *)contextInfo;
{
	[alert release];

	if (returnCode == NSAlertSecondButtonReturn)
		return;
	if (returnCode == NSAlertFirstButtonReturn) {
		[self saveAllFiles:nil];
		[self close]; // HERE'S THE CULPRIT
	}
	if (returnCode == NSAlertThirdButtonReturn)
		[self close]; // close without saving
}

- (void)canCloseDocumentWithDelegate:(id)delegate shouldCloseSelector:(SEL)shouldCloseSelector contextInfo:(void *)contextInfo;
{
	NSLog(@"%p %s",self,__func__);
	BOOL canClose = YES;
	
	for (ESNode *node in self.projectFiles) {
		if (node.isEdited) {
			canClose = NO;
			break;
		}
	}
	
	if (!canClose) {
		NSAlert *alert = [[NSAlert alloc] init]; // released in didEndSelector
		[alert setMessageText:NSLocalizedString(@"ThereAreUnsavedFiles",@"")];
		[alert setInformativeText:NSLocalizedString(@"ThereAreUnsavedFileExplanation",@"")];
		[alert addButtonWithTitle:NSLocalizedString(@"SaveAll",@"")];
		[alert addButtonWithTitle:NSLocalizedString(@"Cancel",@"")];
		[alert addButtonWithTitle:NSLocalizedString(@"DontSave",@"")];
		[alert beginSheetModalForWindow:[self windowForSheet] modalDelegate:self didEndSelector:@selector(unsavedFilesAlertDidEnd:returnCode:contextInfo:) contextInfo:nil];
	}
	
	objc_msgSend(delegate,shouldCloseSelector,self,canClose,contextInfo);
}

Thanks in advance, this has got be stumped.

Jon

 

[JonDann]

Sorry about all the smileys! I just noticed and laughed my head off. I love that the majority of Objective-C is a very sad language

 

[kainjow]

You can disable the smilies by editing your post and checking the box "Disable smilies in text". Also put your code in between [code] ... [/code] tags.

I have a feeling the releasing of your alert within the alert's callback is not good. What if you autorelease the alert when you create it and don't manually release it? I've never had to release an alert like that before and I've never had problems with them.

Edit: ok it seems like the problem is when the alert's close selector is called, it calls [self close] which releases the document and deallocates it. But after the alert is finished, you call objc_msgSend() with self as a parameter (not sure why you are doing this...). But self is deallocated so that is why you get the error message I believe.

 

[JonDann]

You can disable the smilies by editing your post and checking the box "Disable smilies in text". Also put your code in between [code] ... [/code] tags.

Thanks! Have done.

I have a feeling the releasing of your alert within the alert's callback is not good. What if you autorelease the alert when you create it and don't manually release it?

I could do that, yeah but in my experience both have worked. I'll change that. (Just did and it didn't fix it)

Edit: ok it seems like the problem is when the alert's close selector is called, it calls [self close] which releases the document and deallocates it. But after the alert is finished, you call objc_msgSend() with self as a parameter (not sure why you are doing this...). But self is deallocated so that is why you get the error message I believe.

The objc_msgSend is actually called immediately after the alert receives -beginSheetModalForWindow: as these begin modal methods return immediately. The objc_msgSend() calls the shouldCloseSelector, which logging showed to be the private -_document:shouldClose:contextInfo: This is what the documentation tells you to do (well they actually tell you to implement your own shouldCloseSelector but I'm not sending the -canCloseDocumentWithDelegate: in the first place, so I'm calling the selector and delegate that whatever mysterious object sent when invoking the method). So the alert is running and all that objc_msgSend() line does is call the shouldCloseSelector with NO, so the window stays open -essentially it does nothing!

When the user then hits the NSAlertFirstButtonReturn, the files are saved and the [self close] doesn't call -canCloseDocumentWithDelegate: so the objc_msgSend() I think is ok.

It seems that the [self saveDocument:nil]; in -saveAllFiles: is still doing its magic when the first [self close] in the -unsavedFilesAlertDidEnd: is called.

Thanks for your input on this, I really need the help!

 

[JonDann]

Edit: ok it seems like the problem is when the alert's close selector is called, it calls [self close] which releases the document and deallocates it. But after the alert is finished, you call objc_msgSend() with self as a parameter (not sure why you are doing this...). But self is deallocated so that is why you get the error message I believe.

Thinking about it again, you may be right.

If the beginSheetModalForWindow returns only when the alert has finished its transition (i.e. not quite as immediately as I was thinking), then if the user quickly saves while the transition is going on then the [self close] can get sent before the -beginSheetModalForWindow has exited and the objc_msgSend will then be sent with NO and self when the close is in process, or has finished already!

If the user waits for the transition to finish, then the objc_msgSend() has already done its stuff before the user selects what they want to do and all is well with the world!

You sir, are a genius. I really do appreciate your help on this, I'll post how I fix this when I've managed to, but now its breakfast time).

Edit: Ok so I tried the simple (but hackish) fix of removing the call of objc_msgSend(). I also wrapped it in

if (self)
objc_msgSend(delegate,shouldCloseSelector,self,canClose,contextInfo);

and the crash is still there. So its not that line that's the problem. I'm quite convinced that NSDocument's -saveDocument: method is spawning a new thread to do the save and returning before the save is complete, so the -close method starts its job while the saving is still in process.

Any more ideas?

 

[JonDann]

Solved

It turns out (thanks Scott Stevenson) that NSDocument's -saveDocument: method will perform most of its magic after a delay and gets called after the current user event.

So the fix is to call -writeToURLfType:error: which will do it's thing immediately rather than -saveDocument. This means that it is no longer trying to save when -close is called.

Apparently there are a few actions that do their thing in a delayed manner... something to watch out for maybe?