From 1b63746799cc2966570c1db8f4641fc6c23e63bf Mon Sep 17 00:00:00 2001 From: Richard Frith-MacDonald Date: Wed, 22 Jun 2016 09:09:29 +0000 Subject: [PATCH] Thread-safety fixes. git-svn-id: svn+ssh://svn.gna.org/svn/gnustep/libs/base/trunk@39902 72102866-910b-0410-8b05-ffd578937521 --- ChangeLog | 2 + Headers/Foundation/NSDistributedLock.h | 7 +- Source/NSDistributedLock.m | 232 +++++++++++++++---------- 3 files changed, 148 insertions(+), 93 deletions(-) diff --git a/ChangeLog b/ChangeLog index d1c8dce5d..3c721ec0a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,8 @@ * Source/NSOperation.m: When starting an operation, have it retain itself in case it'ss removed from the queue and released while running. + * Headers/Foundation/NSDistributedLock.h: Add lock ivar. + * Source/NSDistributedLock.m: Make class thread-safe using lock. 2016-06-19 Richard Frith-Macdonald diff --git a/Headers/Foundation/NSDistributedLock.h b/Headers/Foundation/NSDistributedLock.h index 78263d3d8..088f56a64 100644 --- a/Headers/Foundation/NSDistributedLock.h +++ b/Headers/Foundation/NSDistributedLock.h @@ -27,8 +27,10 @@ #import #import -#import -#import + +@class NSDate; +@class NSLock; +@class NSString; #if defined(__cplusplus) extern "C" { @@ -39,6 +41,7 @@ extern "C" { #if GS_EXPOSE(NSDistributedLock) NSString *_lockPath; NSDate *_lockTime; + NSLock *_localLock; #endif #if GS_NONFRAGILE #else diff --git a/Source/NSDistributedLock.m b/Source/NSDistributedLock.m index 029ac4e22..1f5ed6528 100644 --- a/Source/NSDistributedLock.m +++ b/Source/NSDistributedLock.m @@ -28,8 +28,9 @@ #import "common.h" #define EXPOSE_NSDistributedLock_IVARS 1 #import "Foundation/NSDistributedLock.h" -#import "Foundation/NSFileManager.h" #import "Foundation/NSException.h" +#import "Foundation/NSFileManager.h" +#import "Foundation/NSLock.h" #import "Foundation/NSValue.h" #import "GSPrivate.h" @@ -74,27 +75,39 @@ static NSFileManager *mgr = nil; */ - (void) breakLock { - NSDictionary *attributes; - - DESTROY(_lockTime); - attributes = [mgr fileAttributesAtPath: _lockPath traverseLink: YES]; - if (attributes != nil) + [_localLock lock]; + NS_DURING { - NSDate *modDate = [attributes fileModificationDate]; + NSDictionary *attributes; - if ([mgr removeFileAtPath: _lockPath handler: nil] == NO) + DESTROY(_lockTime); + attributes = [mgr fileAttributesAtPath: _lockPath traverseLink: YES]; + if (attributes != nil) { - NSString *err = [[NSError _last] localizedDescription]; + NSDate *modDate = [attributes fileModificationDate]; - attributes = [mgr fileAttributesAtPath: _lockPath traverseLink: YES]; - if ([modDate isEqual: [attributes fileModificationDate]] == YES) + if ([mgr removeFileAtPath: _lockPath handler: nil] == NO) { - [NSException raise: NSGenericException - format: @"Failed to remove lock directory '%@' - %@", - _lockPath, err]; + NSString *err = [[NSError _last] localizedDescription]; + + attributes = [mgr fileAttributesAtPath: _lockPath + traverseLink: YES]; + if ([modDate isEqual: [attributes fileModificationDate]] == YES) + { + [NSException raise: NSGenericException + format: @"Failed to remove lock directory '%@' - %@", + _lockPath, err]; + } } } } + NS_HANDLER + { + [_localLock unlock]; + [localException raise]; + } + NS_ENDHANDLER + [_localLock unlock]; } - (void) dealloc @@ -107,21 +120,27 @@ static NSFileManager *mgr = nil; } RELEASE(_lockPath); RELEASE(_lockTime); + RELEASE(_localLock); [super dealloc]; } - (NSString*) description { + NSString *result; + + [_localLock lock]; if (_lockTime == nil) { - return [[super description] stringByAppendingFormat: + result = [[super description] stringByAppendingFormat: @" path '%@' not locked", _lockPath]; } else { - return [[super description] stringByAppendingFormat: + result = [[super description] stringByAppendingFormat: @" path '%@' locked at %@", _lockPath, _lockTime]; } + [_localLock unlock]; + return result; } /** @@ -137,6 +156,7 @@ static NSFileManager *mgr = nil; NSString *lockDir; BOOL isDirectory; + _localLock = [NSLock new]; _lockPath = [[aPath stringByStandardizingPath] copy]; _lockTime = nil; @@ -189,59 +209,79 @@ static NSFileManager *mgr = nil; */ - (BOOL) tryLock { - NSMutableDictionary *attributesToSet; - NSDictionary *attributes; - BOOL locked; + BOOL locked = NO; - attributesToSet = [NSMutableDictionary dictionaryWithCapacity: 1]; - [attributesToSet setObject: [NSNumber numberWithUnsignedInt: 0755] - forKey: NSFilePosixPermissions]; - - locked = [mgr createDirectoryAtPath: _lockPath - withIntermediateDirectories: YES - attributes: attributesToSet - error: NULL]; - if (locked == NO) + [_localLock lock]; + NS_DURING { - BOOL dir; + NSMutableDictionary *attributesToSet; + NSDictionary *attributes; - /* - * We expect the directory creation to have failed because it already - * exists as another processes lock. If the directory doesn't exist, - * then either the other process has removed it's lock (and we can retry) - * or we have a severe problem! - */ - if ([mgr fileExistsAtPath: _lockPath isDirectory: &dir] == NO) + if (nil != _lockTime) { - locked = [mgr createDirectoryAtPath: _lockPath - withIntermediateDirectories: YES - attributes: attributesToSet - error: NULL]; - if (locked == NO) + [NSException raise: NSGenericException + format: @"Attempt to re-lock distributed lock %@", + _lockPath]; + } + attributesToSet = [NSMutableDictionary dictionaryWithCapacity: 1]; + [attributesToSet setObject: [NSNumber numberWithUnsignedInt: 0755] + forKey: NSFilePosixPermissions]; + + locked = [mgr createDirectoryAtPath: _lockPath + withIntermediateDirectories: YES + attributes: attributesToSet + error: NULL]; + if (NO == locked) + { + BOOL dir; + + /* We expect the directory creation to have failed because + * it already exists as another processes lock. + * If the directory doesn't exist, then either the other + * process has removed it's lock (and we can retry) + * or we have a severe problem! + */ + if ([mgr fileExistsAtPath: _lockPath isDirectory: &dir] == NO) { - NSLog(@"Failed to create lock directory '%@' - %@", - _lockPath, [NSError _last]); + locked = [mgr createDirectoryAtPath: _lockPath + withIntermediateDirectories: YES + attributes: attributesToSet + error: NULL]; + if (NO == locked) + { + NSLog(@"Failed to create lock directory '%@' - %@", + _lockPath, [NSError _last]); + } + } + } + + if (YES == locked) + { + attributes = [mgr fileAttributesAtPath: _lockPath + traverseLink: YES]; + if (attributes == nil) + { + [NSException raise: NSGenericException + format: @"Unable to get attributes of lock file we made at %@", + _lockPath]; + } + ASSIGN(_lockTime, [attributes fileModificationDate]); + if (nil == _lockTime) + { + [NSException raise: NSGenericException + format: @"Unable to get date of lock file we made at %@", + _lockPath]; } } } - - if (locked == NO) + NS_HANDLER { - return NO; - } - else - { - attributes = [mgr fileAttributesAtPath: _lockPath - traverseLink: YES]; - if (attributes == nil) - { - [NSException raise: NSGenericException - format: @"Unable to get attributes of lock file we made at %@", - _lockPath]; - } - ASSIGN(_lockTime, [attributes fileModificationDate]); - return YES; + [_localLock unlock]; + [localException raise]; } + NS_ENDHANDLER + [_localLock unlock]; + return locked; } /** @@ -251,43 +291,53 @@ static NSFileManager *mgr = nil; */ - (void) unlock { - NSDictionary *attributes; + [_localLock lock]; + NS_DURING + { + NSDictionary *attributes; - if (_lockTime == nil) - { - [NSException raise: NSGenericException format: @"not locked by us"]; - } - - /* - * Don't remove the lock if it has already been broken by someone - * else and re-created. Unfortunately, there is a window between - * testing and removing, but we do the bset we can. - */ - attributes = [mgr fileAttributesAtPath: _lockPath traverseLink: YES]; - if (attributes == nil) - { - DESTROY(_lockTime); - [NSException raise: NSGenericException - format: @"lock '%@' already broken", _lockPath]; - } - if ([_lockTime isEqual: [attributes fileModificationDate]]) - { - DESTROY(_lockTime); - if ([mgr removeFileAtPath: _lockPath handler: nil] == NO) + if (_lockTime == nil) { - [NSException raise: NSGenericException - format: @"Failed to remove lock directory '%@' - %@", - _lockPath, [NSError _last]]; + [NSException raise: NSGenericException format: @"not locked by us"]; + } + + /* Don't remove the lock if it has already been broken by someone + * else and re-created. Unfortunately, there is a window between + * testing and removing, but we do the bset we can. + */ + attributes = [mgr fileAttributesAtPath: _lockPath traverseLink: YES]; + if (attributes == nil) + { + DESTROY(_lockTime); + [NSException raise: NSGenericException + format: @"lock '%@' already broken", _lockPath]; + } + if ([_lockTime isEqual: [attributes fileModificationDate]]) + { + DESTROY(_lockTime); + if ([mgr removeFileAtPath: _lockPath handler: nil] == NO) + { + [NSException raise: NSGenericException + format: @"Failed to remove lock directory '%@' - %@", + _lockPath, [NSError _last]]; + } + } + else + { + DESTROY(_lockTime); + [NSException raise: NSGenericException + format: @"lock '%@' already broken and in use again", + _lockPath]; } - } - else - { DESTROY(_lockTime); - [NSException raise: NSGenericException - format: @"lock '%@' already broken and in use again", - _lockPath]; } - DESTROY(_lockTime); + NS_HANDLER + { + [_localLock unlock]; + [localException raise]; + } + NS_ENDHANDLER + [_localLock unlock]; } @end