From 5a31e9ba5951dd8aa21bf11af550616a427abada Mon Sep 17 00:00:00 2001 From: Frederik Seiffert Date: Wed, 19 Jul 2023 12:33:15 +0200 Subject: [PATCH] Fix NSFileManager thread safety. --- ChangeLog | 6 +++ Headers/Foundation/NSFileManager.h | 1 + Source/NSFileManager.m | 86 ++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+) diff --git a/ChangeLog b/ChangeLog index e49acf8220..0212973050 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2023-07-19 Frederik Seiffert + + * Headers/Foundation/NSFileManager.h: + * Source/NSFileManager.m: + Fix NSFileManager thread safety. + 2023-06-10 Riccardo Mottola * Tests/base/NSURL/Helpers/Launch.h: diff --git a/Headers/Foundation/NSFileManager.h b/Headers/Foundation/NSFileManager.h index 7492883a41..cdab296ecd 100644 --- a/Headers/Foundation/NSFileManager.h +++ b/Headers/Foundation/NSFileManager.h @@ -217,6 +217,7 @@ GS_EXPORT_CLASS @private id _delegate; NSString *_lastError; + gs_mutex_public_t _lastErrorMutex; #endif #if GS_NONFRAGILE #else diff --git a/Source/NSFileManager.m b/Source/NSFileManager.m index 166bb4d7eb..5ad3e4bceb 100644 --- a/Source/NSFileManager.m +++ b/Source/NSFileManager.m @@ -60,6 +60,7 @@ #import "Foundation/NSURL.h" #import "Foundation/NSValue.h" #import "GSPrivate.h" +#import "GSPThread.h" #import "GNUstepBase/NSString+GNUstepBase.h" #import "GNUstepBase/NSTask+GNUstepBase.h" @@ -360,9 +361,20 @@ + (void) initialize GSAttrDictionaryClass = [GSAttrDictionary class]; } +- (id) init +{ + if (nil != (self = [super init])) + { + GS_MUTEX_INIT(_lastErrorMutex); + } + return self; +} + - (void) dealloc { + GS_MUTEX_LOCK(_lastErrorMutex); TEST_RELEASE(_lastError); + GS_MUTEX_UNLOCK(_lastErrorMutex); [super dealloc]; } @@ -439,7 +451,9 @@ - (BOOL) changeFileAttributes: (NSDictionary*)attributes atPath: (NSString*)path str = [NSString stringWithFormat: @"Unable to change NSFileOwnerAccountID to '%"PRIuPTR"' - %@", num, [NSError _last]]; + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, str); + GS_MUTEX_UNLOCK(_lastErrorMutex); } } else @@ -481,7 +495,9 @@ - (BOOL) changeFileAttributes: (NSDictionary*)attributes atPath: (NSString*)path str = [NSString stringWithFormat: @"Unable to change NSFileOwnerAccountName to '%@' - %@", str, [NSError _last]]; + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, str); + GS_MUTEX_UNLOCK(_lastErrorMutex); } } } @@ -505,7 +521,9 @@ - (BOOL) changeFileAttributes: (NSDictionary*)attributes atPath: (NSString*)path str = [NSString stringWithFormat: @"Unable to change NSFileGroupOwnerAccountID to '%"PRIuPTR"' - %@", num, [NSError _last]]; + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, str); + GS_MUTEX_UNLOCK(_lastErrorMutex); } } else if ((str = [attributes fileGroupOwnerAccountName]) != nil @@ -545,7 +563,9 @@ - (BOOL) changeFileAttributes: (NSDictionary*)attributes atPath: (NSString*)path str = [NSString stringWithFormat: @"Unable to change NSFileGroupOwnerAccountName to '%@' - %@", str, [NSError _last]]; + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, str); + GS_MUTEX_UNLOCK(_lastErrorMutex); } } #endif /* _WIN32 */ @@ -559,7 +579,9 @@ - (BOOL) changeFileAttributes: (NSDictionary*)attributes atPath: (NSString*)path str = [NSString stringWithFormat: @"Unable to change NSFilePosixPermissions to '%o' - %@", (unsigned)num, [NSError _last]]; + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, str); + GS_MUTEX_UNLOCK(_lastErrorMutex); } } @@ -629,7 +651,9 @@ - (BOOL) changeFileAttributes: (NSDictionary*)attributes atPath: (NSString*)path str = [NSString stringWithFormat: @"Unable to change NSFileCreationDate to '%@' - %@", date, [NSError _last]]; + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, str); + GS_MUTEX_UNLOCK(_lastErrorMutex); } } @@ -678,7 +702,9 @@ - (BOOL) changeFileAttributes: (NSDictionary*)attributes atPath: (NSString*)path str = [NSString stringWithFormat: @"Unable to change NSFileModificationDate to '%@' - %@", date, [NSError _last]]; + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, str); + GS_MUTEX_UNLOCK(_lastErrorMutex); } } @@ -805,7 +831,9 @@ - (NSArray*) contentsOfDirectoryAtURL: (NSURL*)url NSDirectoryEnumerator *direnum; NSString *path; + GS_MUTEX_LOCK(_lastErrorMutex); DESTROY(_lastError); + GS_MUTEX_UNLOCK(_lastErrorMutex); if (![[url scheme] isEqualToString: @"file"]) { @@ -909,7 +937,9 @@ - (NSDirectoryEnumerator *)enumeratorAtURL: (NSURL *)url BOOL shouldRecurse; BOOL shouldSkipHidden; + GS_MUTEX_LOCK(_lastErrorMutex); DESTROY(_lastError); + GS_MUTEX_UNLOCK(_lastErrorMutex); if (![[url scheme] isEqualToString: @"file"]) { @@ -953,7 +983,9 @@ - (NSArray*) contentsOfDirectoryAtPath: (NSString*)path error: (NSError**)error { NSArray *result; + GS_MUTEX_LOCK(_lastErrorMutex); DESTROY(_lastError); + GS_MUTEX_UNLOCK(_lastErrorMutex); result = [self directoryContentsAtPath: path]; if (error != NULL) @@ -982,7 +1014,9 @@ - (BOOL) createDirectoryAtPath: (NSString *)path { BOOL result = NO; + GS_MUTEX_LOCK(_lastErrorMutex); DESTROY(_lastError); + GS_MUTEX_UNLOCK(_lastErrorMutex); if (YES == flag) { NSEnumerator *paths = [[path pathComponents] objectEnumerator]; @@ -1014,7 +1048,9 @@ - (BOOL) createDirectoryAtPath: (NSString *)path else { result = NO; + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"Could not create directory - intermediate path did not exist or was not a directory"); + GS_MUTEX_UNLOCK(_lastErrorMutex); } } @@ -1062,7 +1098,9 @@ - (BOOL) createDirectoryAtPath: (NSString*)path /* This is consistent with MacOSX - just return NO for an invalid path. */ if ([path length] == 0) { + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"no path given"); + GS_MUTEX_UNLOCK(_lastErrorMutex); return NO; } @@ -1080,7 +1118,9 @@ - (BOOL) createDirectoryAtPath: (NSString*)path e = [NSString stringWithFormat: @"path %@ exists ... cannot create", path]; } + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, e); + GS_MUTEX_UNLOCK(_lastErrorMutex); return NO; } else @@ -1125,7 +1165,9 @@ - (BOOL) createDirectoryAtPath: (NSString*)path e = [NSString stringWithFormat: @"Could not create '%@' - '%@'", path, [NSError _last]]; + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, e); + GS_MUTEX_UNLOCK(_lastErrorMutex); return NO; } } @@ -1156,7 +1198,9 @@ - (BOOL) createFileAtPath: (NSString*)path /* This is consistent with MacOSX - just return NO for an invalid path. */ if ([path length] == 0) { + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"no path given"); + GS_MUTEX_UNLOCK(_lastErrorMutex); return NO; } @@ -1322,8 +1366,10 @@ - (BOOL) copyPath: (NSString*)source if ([[destination stringByAppendingString: @"/"] hasPrefix: [source stringByAppendingString: @"/"]]) { + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"Could not copy - destination is a descendant of source"); + GS_MUTEX_UNLOCK(_lastErrorMutex); return NO; } @@ -1385,7 +1431,9 @@ - (BOOL) copyItemAtPath: (NSString*)src { BOOL result; + GS_MUTEX_LOCK(_lastErrorMutex); DESTROY(_lastError); + GS_MUTEX_UNLOCK(_lastErrorMutex); result = [self copyPath: src toPath: dst handler: nil]; if (error != NULL) @@ -1455,7 +1503,9 @@ - (BOOL) movePath: (NSString*)source if (sourceIsDir && [[destination stringByAppendingString: @"/"] hasPrefix: [source stringByAppendingString: @"/"]]) { + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"Could not move - destination is a descendant of source"); + GS_MUTEX_UNLOCK(_lastErrorMutex); return NO; } @@ -1499,7 +1549,9 @@ - (BOOL) moveItemAtPath: (NSString*)src { BOOL result; + GS_MUTEX_LOCK(_lastErrorMutex); DESTROY(_lastError); + GS_MUTEX_UNLOCK(_lastErrorMutex); result = [self movePath: src toPath: dst handler: nil]; if (error != NULL) @@ -1570,7 +1622,9 @@ - (BOOL) linkPath: (NSString*)source if ([[destination stringByAppendingString: @"/"] hasPrefix: [source stringByAppendingString: @"/"]]) { + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"Could not link - destination is a descendant of source"); + GS_MUTEX_UNLOCK(_lastErrorMutex); return NO; } @@ -1624,7 +1678,9 @@ - (BOOL) linkPath: (NSString*)source [self changeFileAttributes: attrs atPath: destination]; return YES; #else + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"Links not supported on this platform"); + GS_MUTEX_UNLOCK(_lastErrorMutex); return NO; #endif } @@ -1646,7 +1702,9 @@ - (BOOL) removeFileAtPath: (NSString*)path lpath = [self fileSystemRepresentationWithPath: path]; if (lpath == 0 || *lpath == 0) { + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"Could not remove - no path"); + GS_MUTEX_UNLOCK(_lastErrorMutex); return NO; } else @@ -1741,7 +1799,9 @@ - (BOOL) removeItemAtPath: (NSString*)path { BOOL result; + GS_MUTEX_LOCK(_lastErrorMutex); DESTROY(_lastError); + GS_MUTEX_UNLOCK(_lastErrorMutex); result = [self removeFileAtPath: path handler: nil]; if (error != NULL) @@ -1767,7 +1827,9 @@ - (BOOL) createSymbolicLinkAtPath: (NSString*)path { BOOL result; + GS_MUTEX_LOCK(_lastErrorMutex); DESTROY(_lastError); + GS_MUTEX_UNLOCK(_lastErrorMutex); result = [self createSymbolicLinkAtPath: path pathContent: destPath]; if (error != NULL) @@ -1797,7 +1859,9 @@ - (BOOL) fileExistsAtPath: (NSString*)path isDirectory: (BOOL*)isDirectory if (lpath == 0 || *lpath == _NUL) { + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"no path given"); + GS_MUTEX_UNLOCK(_lastErrorMutex); return NO; } @@ -1875,7 +1939,9 @@ - (BOOL) isReadableFileAtPath: (NSString*)path if (lpath == 0 || *lpath == _NUL) { + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"no path given"); + GS_MUTEX_UNLOCK(_lastErrorMutex); return NO; } @@ -1932,7 +1998,9 @@ - (BOOL) isWritableFileAtPath: (NSString*)path if (lpath == 0 || *lpath == _NUL) { + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"no path given"); + GS_MUTEX_UNLOCK(_lastErrorMutex); return NO; } @@ -1974,7 +2042,9 @@ - (BOOL) isExecutableFileAtPath: (NSString*)path if (lpath == 0 || *lpath == _NUL) { + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"no path given"); + GS_MUTEX_UNLOCK(_lastErrorMutex); return NO; } @@ -2033,7 +2103,9 @@ - (BOOL) isDeletableFileAtPath: (NSString*)path if (lpath == 0 || *lpath == _NUL) { + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"no path given"); + GS_MUTEX_UNLOCK(_lastErrorMutex); return NO; } @@ -2227,7 +2299,9 @@ - (NSDictionary*) attributesOfItemAtPath: (NSString*)path { NSDictionary *d; + GS_MUTEX_LOCK(_lastErrorMutex); DESTROY(_lastError); + GS_MUTEX_UNLOCK(_lastErrorMutex); d = [GSAttrDictionaryClass attributesAt: path traverseLink: NO]; if (error != NULL) @@ -2368,7 +2442,9 @@ - (NSDictionary*) attributesOfFileSystemForPath: (NSString*)path return [NSDictionary dictionaryWithObjects: values forKeys: keys count: 5]; #else GSOnceMLog(@"NSFileManager", @"no support for filesystem attributes"); + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"no support for filesystem attributes"); + GS_MUTEX_UNLOCK(_lastErrorMutex); return nil; #endif #endif /* _WIN32 */ @@ -2522,7 +2598,9 @@ - (BOOL) createSymbolicLinkAtPath: (NSString*)path return (symlink(oldpath, newpath) == 0); #else + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"symbolic links not supported on this system"); + GS_MUTEX_UNLOCK(_lastErrorMutex); return NO; #endif } @@ -2547,7 +2625,9 @@ - (NSString*) pathContentOfSymbolicLinkAtPath: (NSString*)path return nil; } #else + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"symbolic links not supported on this system"); + GS_MUTEX_UNLOCK(_lastErrorMutex); return nil; #endif } @@ -3398,7 +3478,9 @@ - (BOOL) _copyPath: (NSString*)source s = [NSString stringWithFormat: @"cannot copy file type '%@'", fileType]; + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, s); + GS_MUTEX_UNLOCK(_lastErrorMutex); NSDebugLog(@"%@: %@", sourceFile, s); continue; } @@ -3502,7 +3584,9 @@ - (BOOL) _linkPath: (NSString*)source LEAVE_POOL return result; #else + GS_MUTEX_LOCK(_lastErrorMutex); ASSIGN(_lastError, @"Links not supported on this platform"); + GS_MUTEX_UNLOCK(_lastErrorMutex); return NO; #endif } @@ -3560,6 +3644,7 @@ - (NSError*) _errorFrom: (NSString *)fromPath to: (NSString *)toPath NSString *domain; NSInteger code; + GS_MUTEX_LOCK(_lastErrorMutex); if (_lastError) { message = _lastError; @@ -3600,6 +3685,7 @@ - (NSError*) _errorFrom: (NSString *)fromPath to: (NSString *)toPath code: code userInfo: errorInfo]; DESTROY(_lastError); + GS_MUTEX_UNLOCK(_lastErrorMutex); return error; }