Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Experimental/autosave freeze fix #25

Open
wants to merge 8 commits into
base: v0.3.x
Choose a base branch
from
Open

Experimental/autosave freeze fix #25

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a216aea
Do our own AutosaveInPlace operation.
adib May 17, 2014
7f04882
Ensure that completion handlers are executed on the main queue.
adib May 17, 2014
0a921c6
Make sure that we set the modification date as the last thing.
adib May 17, 2014
82dc52a
We'll need to nullify the autosaved contents URL here
adib May 28, 2014
4af1fdb
Added debug log statement to indicate the save operation type.
adib Jul 11, 2014
6ac90c2
Guard the _content instance variable by borrowing the parent context's
adib Jul 11, 2014
15f9a11
Improved identification of unknown save operation types.
adib Jul 28, 2014
0d74cbb
Work around NSAutosaveElsewhereOperation if the autosave name clashes.
adib Jul 29, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
219 changes: 175 additions & 44 deletions BSManagedDocument.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#import "BSManagedDocument.h"

static NSString* BSStringFromSaveOperationType(NSSaveOperationType type);

@interface BSManagedDocument ()
@property(nonatomic, copy) NSURL *autosavedContentsTempDirectoryURL;
Expand Down Expand Up @@ -563,28 +564,45 @@ - (void)saveToURL:(NSURL *)url ofType:(NSString *)typeName forSaveOperation:(NSS
// * If autosaving while quitting, calling -performActivity… here results in deadlock
[self performAsynchronousFileAccessUsingBlock:^(void (^fileAccessCompletionHandler)(void)) {

NSAssert(_contents == nil, @"Can't begin save; another is already in progress. Perhaps you forgot to wrap the call inside of -performActivityWithSynchronousWaiting:usingBlock:");


// Stash contents temporarily into an ivar so -writeToURL:… can access it from the worker thread
NSError *error;
_contents = [self contentsForURL:url ofType:typeName saveOperation:saveOperation error:&error];
// guard _contents from modification
NSManagedObjectContext* parentContext = [self.managedObjectContext parentContext];
BOOL __block hasContents = NO;
NSError* __block returnError = nil;
[parentContext performBlockAndWait:^{
NSAssert(_contents == nil, @"Can't begin save; another is already in progress. Perhaps you forgot to wrap the call inside of -performActivityWithSynchronousWaiting:usingBlock:");
// Stash contents temporarily into an ivar so -writeToURL:… can access it from the worker thread
NSError *error = nil;
_contents = [self contentsForURL:url ofType:typeName saveOperation:saveOperation error:&error];
if (error) {
returnError = error;
}
#if !__has_feature(objc_arc)
[_contents retain];
#endif
hasContents = _contents != nil;
}];

if (!_contents)
if (!hasContents)
{
// The docs say "be sure to invoke super", but by my understanding it's fine not to if it's because of a failure, as the filesystem hasn't been touched yet.
fileAccessCompletionHandler();
if (completionHandler) completionHandler(error);
return;
}

if (completionHandler)
{
#if !__has_feature(objc_arc)
returnError = [returnError retain];
#endif
dispatch_async(dispatch_get_main_queue(), ^{
#if !__has_feature(objc_arc)
[_contents retain];
[returnError autorelease];
#endif
completionHandler(returnError);
});
}
return;
}


// Kick off async saving work
[super saveToURL:url ofType:typeName forSaveOperation:saveOperation completionHandler:^(NSError *error) {
void(^saveCompletionHandler)(NSError*) =^(NSError *error) {

// If the save failed, it might be an error the user can recover from.
// e.g. the dreaded "file modified by another application"
Expand All @@ -599,21 +617,24 @@ - (void)saveToURL:(NSURL *)url ofType:(NSString *)typeName forSaveOperation:(NSS
if ([error recoveryAttempter])
{
[self performActivityWithSynchronousWaiting:NO usingBlock:^(void (^activityCompletionHandler)(void)) {

[parentContext performBlockAndWait:^{
#if !__has_feature(objc_arc)
[_contents release];
[_contents release];
#endif
_contents = nil;
_contents = nil;
}];

activityCompletionHandler();
}];
}
else
{
[parentContext performBlockAndWait:^{
#if !__has_feature(objc_arc)
[_contents release];
[_contents release];
#endif
_contents = nil;
_contents = nil;
}];
}


Expand All @@ -627,8 +648,80 @@ - (void)saveToURL:(NSURL *)url ofType:(NSString *)typeName forSaveOperation:(NSS

// And can finally declare we're done
fileAccessCompletionHandler();
if (completionHandler) completionHandler(error);
}];
if (completionHandler)
{
dispatch_async(dispatch_get_main_queue(), ^{
completionHandler(error);
});
}
};
#ifdef DDLogDebug
DDLogDebug(@"Starting saving work with operation: %@",BSStringFromSaveOperationType(saveOperation));
#endif
// Kick off async saving work
if (saveOperation == NSAutosaveInPlaceOperation)
{
// NSDocument's implementation for AutosaveInPlace seems to be broken and freezes on NSFileCoordinator.
// at [NSDocument _fileCoordinator:coordinateReadingContentsAndWritingItemAtURL:byAccessor:] to be precise
// thus we do our own just for this.
NSManagedObjectContext* parentContext = [self.managedObjectContext parentContext];
void(^autosaveInPlaceImplementation)() = ^{
[parentContext performBlockAndWait:^{
NSError* fileWriteError = nil;
if ([self writeSafelyToURL:url ofType:typeName forSaveOperation:saveOperation error:&fileWriteError])
{
[self setFileType:typeName];
[self setFileURL:url];
[self setAutosavedContentsFileURL:nil];
[self updateChangeCount:NSChangeAutosaved];

NSDate* fileDate = nil;
[url getResourceValue:&fileDate forKey:NSURLAttributeModificationDateKey error:nil];
if (!fileDate) {
fileDate = [NSDate date];
}
[self setFileModificationDate:fileDate];
saveCompletionHandler(nil);
}
else
{
saveCompletionHandler(fileWriteError);
}
}];
};
if ([self canAsynchronouslyWriteToURL:url ofType:typeName forSaveOperation:saveOperation])
{
[[self presentedItemOperationQueue] addOperationWithBlock:autosaveInPlaceImplementation];
}
else
{
autosaveInPlaceImplementation();
}
}
else
{
if (saveOperation == NSAutosaveElsewhereOperation) {
NSURL* originalAutosavedContentsFileURL = self.autosavedContentsFileURL;
NSURL* autosavedContentsFileURL = originalAutosavedContentsFileURL;
int counter = 0;
// Make sure that we have a non-pre-existing file as the autosave contents.
while ([autosavedContentsFileURL checkResourceIsReachableAndReturnError:nil]) {
counter++;
NSString* path = [[autosavedContentsFileURL path] stringByStandardizingPath];
NSString* file = [path lastPathComponent];
NSString* dir = [path stringByDeletingLastPathComponent];
NSString* baseName = [file stringByDeletingPathExtension];
NSString* extension = [file pathExtension];
NSString* updatedName = [NSString stringWithFormat:@"%@ %d.%@",baseName,counter,extension];
autosavedContentsFileURL = [NSURL fileURLWithPath:[dir stringByAppendingPathComponent:updatedName]];
}
[self setAutosavedContentsFileURL:autosavedContentsFileURL];
#ifdef DDLogDebug
DDLogDebug(@"Autosave elsewhere. autosavedContentsFileURL: %@ originalAutosavedContentsFileURL: %@",autosavedContentsFileURL,originalAutosavedContentsFileURL);
#endif
}
[super saveToURL:url ofType:typeName forSaveOperation:saveOperation completionHandler:saveCompletionHandler];
}
}];
}

Expand Down Expand Up @@ -751,27 +844,37 @@ - (BOOL)writeToURL:(NSURL *)inURL
// Grab additional content before proceeding. This should *only* happen when writing entirely on the main thread
// (e.g. Using one of the old synchronous -save… APIs. Note: duplicating a document calls -writeSafely… directly)
// To have gotten here on any thread but the main one is a programming error and unworkable, so we throw an exception
if (!_contents)
{
_contents = [self contentsForURL:inURL ofType:typeName saveOperation:saveOp error:error];
if (!_contents) return NO;

// Worried that _contents hasn't been retained? Never fear, we'll set it straight back to nil before exiting this method, I promise


// And now we're ready to write for real
BOOL result = [self writeToURL:inURL ofType:typeName forSaveOperation:saveOp originalContentsURL:originalContentsURL error:error];


// Finish up. Don't worry, _additionalContent was never retained on this codepath, so doesn't need to be released
_contents = nil;
return result;
}
BOOL __block result = NO;
NSManagedObjectContext* parentContext = [self.managedObjectContext parentContext];
[parentContext performBlockAndWait:^{
if (!_contents)
{
_contents = [self contentsForURL:inURL ofType:typeName saveOperation:saveOp error:error];
if (!_contents) {
result = NO;
return;
}

// Worried that _contents hasn't been retained? Never fear, we'll set it straight back to nil before exiting this method, I promise


// And now we're ready to write for real
result = [self writeToURL:inURL ofType:typeName forSaveOperation:saveOp originalContentsURL:originalContentsURL error:error];


// Finish up. Don't worry, _additionalContent was never retained on this codepath, so doesn't need to be released
_contents = nil;
result = NO;
return;
}

// We implement contents as a block which is called to perform the writing
BOOL (^contentsBlock)(NSURL *, NSSaveOperationType, NSURL *, NSError**) = _contents;
result = contentsBlock(inURL, saveOp, originalContentsURL, error);
}];

return result;

// We implement contents as a block which is called to perform the writing
BOOL (^contentsBlock)(NSURL *, NSSaveOperationType, NSURL *, NSError**) = _contents;
return contentsBlock(inURL, saveOp, originalContentsURL, error);
}

- (void)setBundleBitForDirectoryAtURL:(NSURL *)url;
Expand Down Expand Up @@ -1010,10 +1113,13 @@ - (NSDocument *)duplicateAndReturnError:(NSError **)outError;
BOOL (^contentsBlock)(NSURL*, NSSaveOperationType, NSURL*, NSError**) = ^(NSURL *url, NSSaveOperationType saveOperation, NSURL *originalContentsURL, NSError **error) {
return [self writeBackupToURL:url error:error];
};

_contents = contentsBlock;
NSDocument *result = [super duplicateAndReturnError:outError];
_contents = nil;
NSManagedObjectContext* parentContext = [self.managedObjectContext parentContext];
NSDocument __block* result = nil;
[parentContext performBlockAndWait:^{
_contents = contentsBlock;
result = [super duplicateAndReturnError:outError];
_contents = nil;
}];

return result;
}
Expand Down Expand Up @@ -1107,3 +1213,28 @@ - (NSError *)willPresentError:(NSError *)inError
@end


static NSString* BSStringFromSaveOperationType(NSSaveOperationType type)
{
switch (type) {
case NSSaveOperation:
return @"NSSaveOperation";
case NSSaveAsOperation:
return @"NSSaveAsOperation";
case NSSaveToOperation:
return @"NSSaveToOperation";
#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
case NSAutosaveInPlaceOperation:
return @"NSAutosaveInPlaceOperation";
case NSAutosaveElsewhereOperation:
return @"NSAutosaveElsewhereOperation";
#endif
#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_8
case NSAutosaveAsOperation:
return @"NSAutosaveAsOperation";
#endif
default:
return [NSString stringWithFormat:@"UnknownSaveOperationType:%d",(int)type];
}
}