jerrykrinock / BSManagedDocument

Brings UIManagedDocument's design to OS X
Other
4 stars 4 forks source link

[NSDocument close] called twice #13

Open evanmiller opened 4 years ago

evanmiller commented 4 years ago

I'm working through some customer issues / crashes that are being triggered because close is calling itself. To be clear, this is interacting with some bugs in my own code, but I think it's safe to say that close should only occur once.

Here's the relevant part of backtrace I'm looking at:

#0  0x0000000100021209 in -[CKMapDocument close] at CKMapDocument.m:2542
#1  0x00007fff3378ac2a in -[NSWindowController _windowDidClose] ()
#2  0x00007fff33c933cd in -[NSWindow _finishClosingWindow] ()
#3  0x00007fff3376ecef in -[NSWindow _close] ()
#4  0x00007fff337d488b in -[NSDocument close] ()
#5  0x00000001000d8be0 in -[BSManagedDocument closeNow] at BSManagedDocument.m:482
#6  0x00000001000d8ade in -[BSManagedDocument close] at BSManagedDocument.m:460
#7  0x0000000100021240 in -[CKMapDocument close] at CKMapDocument.m:2546
#8  0x00007fff339a4991 in __115-[NSDocumentController reviewUnsavedDocumentsWithAlertTitle:cancellable:delegate:didReviewAllSelector:contextInfo:]_block_invoke_3 ()
#9  0x00007fff33824080 in -[NSDocumentController _something:didSomething:soContinue:] ()
#10 0x00000001000dcc19 in __82-[BSManagedDocument canCloseDocumentWithDelegate:shouldCloseSelector:contextInfo:]_block_invoke at BSManagedDocument.m:1605
#11 0x00000001000dcc65 in -[BSManagedDocument document:didDecideToClose:contextInfo:] at BSManagedDocument.m:1628
#12 0x00007fff337e09f8 in __75-[NSDocument canCloseDocumentWithDelegate:shouldCloseSelector:contextInfo:]_block_invoke_2 ()
#13 0x00007fff337e0954 in -[NSDocument _something:didSomething:soContinue:] ()
#14 0x00007fff33efcf83 in __86-[NSDocument(NSDocumentSaving) _saveDocumentWithDelegate:didSaveSelector:contextInfo:]_block_invoke_4 ()
#15 0x00007fff337e0954 in -[NSDocument _something:didSomething:soContinue:] ()
#16 0x00007fff33f003cd in __104-[NSDocument(NSDocumentSaving) _runModalSavePanelForSaveOperation:delegate:didSaveSelector:contextInfo:]_block_invoke.558 ()
#17 0x00007fff337e0954 in -[NSDocument _something:didSomething:soContinue:] ()
#18 0x00007fff3384ef8c in __85-[NSDocument saveToURL:ofType:forSaveOperation:delegate:didSaveSelector:contextInfo:]_block_invoke_3 ()
#19 0x00000001000db094 in __73-[BSManagedDocument saveToURL:ofType:forSaveOperation:completionHandler:]_block_invoke.260 at /Users/emiller/Code/BSManagedDocument.evanmiller/BSManagedDocument.m:1105
#20 0x00007fff33a1cc37 in -[NSDocument(NSDocumentSerializationAPIs) _discontinueFileAccessUsingBlock:] ()
#21 0x00007fff33f03b31 in __85-[NSDocument(NSDocumentSaving) _saveToURL:ofType:forSaveOperation:completionHandler:]_block_invoke.848 ()
#22 0x00007fff33f04fb0 in __85-[NSDocument(NSDocumentSaving) _saveToURL:ofType:forSaveOperation:completionHandler:]_block_invoke_2.903 ()
#23 0x00007fff33f04d7d in __85-[NSDocument(NSDocumentSaving) _saveToURL:ofType:forSaveOperation:completionHandler:]_block_invoke.900 ()
#24 0x00007fff33f03705 in __85-[NSDocument(NSDocumentSaving) _saveToURL:ofType:forSaveOperation:completionHandler:]_block_invoke_3.807 ()
#25 0x00007fff33a1cb47 in -[NSDocument(NSDocumentSerializationAPIs) continueFileAccessUsingBlock:] ()
#26 0x00007fff33f01221 in __119-[NSDocument(NSDocumentSaving) _fileCoordinator:asynchronouslyCoordinateReadingContentsAndWritingItemAtURL:byAccessor:]_block_invoke_2 ()
#27 0x00007fff339ac41b in __62-[NSDocumentController(NSInternal) _onMainThreadInvokeWorker:]_block_invoke_3 ()
#28 0x00007fff339ae9c7 in ___NSMainRunLoopPerformBlockInModes_block_invoke ()
#29 0x00007fff3617abac in __CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__ ()

In my code [CKMapDocument close] calls [super close] (see lines 6 and 7 above). But this ends up triggering another close event after the window is closed.

I will note that I only see this behavior after turning off autosaving / restorable windows in System Preferences (i.e. checking the boxes "Ask to keep changes when closing documents" and "Close windows when quitting an app") but with autosaving turned on in the code (autosavesInPlace returns YES). The crash occurs when attempting to quit the application with an edited document open, as you may have surmised from the backtrace.

I can add a workaround on my end, but I believe there should be some logic added to closeNow to check the _closing flag before initiating another close.

jerrykrinock commented 4 years ago

Sorry, I wanted to look at this today but was dealing with a sewer issue in our house :|

It looks as though you have figured this out, and having -close check _closing as you suggest is indeed what I would do, so it looks like a simple 2-line fix, well, 4 lines if you count curly bracket lines. Please submit a pull request if you want, or I shall get to it next week.

evanmiller commented 4 years ago

Well, I'm not sure the flag would fix it - the entry point on the second call to close is the subclass, so it looks like the subclass would have to check the flag.

Relevant discussion: https://stackoverflow.com/questions/5627267/nsdocument-subclass-close-method-called-twice

Maybe not worth fixing, or impossible to fix on the BSManagedDocument side.

evanmiller commented 4 years ago

Closing issue since I think that double-close is just a fact of life when working with NSDocument

jerrykrinock commented 4 years ago

Life has many interesting facts when NSDocument becomes a part of it :)) So I presume this means you are modifying your subclass to tolerate the second close.

Indeed, this is what I do, for that very reason. I override -close to set a flag and call my finicky "tear down" code only during the first call. Also, my code comments notes that I discovered a few months ago, testing with macOS 10.15.4, that executing the following AppleScript:

tell application "BookMacster" to close every document

indeed causes -close to be called twice.