diff --git a/ChangeLog b/ChangeLog index e49acf822..49cc0035f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2023-07-25 Frederik Seiffert + + * Headers/Foundation/NSFileManager.h: + * Source/NSFileManager.m: + Fixed 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 7492883a4..22c029cac 100644 --- a/Headers/Foundation/NSFileManager.h +++ b/Headers/Foundation/NSFileManager.h @@ -216,7 +216,6 @@ GS_EXPORT_CLASS #if GS_EXPOSE(NSFileManager) @private id _delegate; - NSString *_lastError; #endif #if GS_NONFRAGILE #else diff --git a/Source/NSFileManager.m b/Source/NSFileManager.m index 166bb4d7e..5e8f321c6 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" @@ -269,6 +270,14 @@ + (NSEnumerator*) enumeratorFor: (NSDictionary*)d; @end +gs_thread_key_t thread_last_error_key; + +static void GS_WINAPI +exitedThread(void *lastErrorPtr) +{ + RELEASE(lastErrorPtr); +} + @interface NSFileManager (PrivateMethods) @@ -304,6 +313,10 @@ - (BOOL) _proceedAccordingToHandler: (id) handler fromPath: (NSString*) fromPath toPath: (NSString*) toPath; +/* Thread-local last error variable. */ +- (NSString*) _lastError; +- (void) _setLastError: (NSString*)error; + /* A convenience method to return an NSError object. * If the _lastError message is set, this creates an NSError using * that message in the NSCocoaErrorDomain, otherwise it used the @@ -358,11 +371,12 @@ + (void) initialize { defaultEncoding = [NSString defaultCStringEncoding]; GSAttrDictionaryClass = [GSAttrDictionary class]; + GS_THREAD_KEY_INIT(thread_last_error_key, exitedThread); } - (void) dealloc { - TEST_RELEASE(_lastError); + [self _setLastError: nil]; [super dealloc]; } @@ -439,7 +453,7 @@ - (BOOL) changeFileAttributes: (NSDictionary*)attributes atPath: (NSString*)path str = [NSString stringWithFormat: @"Unable to change NSFileOwnerAccountID to '%"PRIuPTR"' - %@", num, [NSError _last]]; - ASSIGN(_lastError, str); + [self _setLastError: str]; } } else @@ -481,7 +495,7 @@ - (BOOL) changeFileAttributes: (NSDictionary*)attributes atPath: (NSString*)path str = [NSString stringWithFormat: @"Unable to change NSFileOwnerAccountName to '%@' - %@", str, [NSError _last]]; - ASSIGN(_lastError, str); + [self _setLastError: str]; } } } @@ -505,7 +519,7 @@ - (BOOL) changeFileAttributes: (NSDictionary*)attributes atPath: (NSString*)path str = [NSString stringWithFormat: @"Unable to change NSFileGroupOwnerAccountID to '%"PRIuPTR"' - %@", num, [NSError _last]]; - ASSIGN(_lastError, str); + [self _setLastError: str]; } } else if ((str = [attributes fileGroupOwnerAccountName]) != nil @@ -545,7 +559,7 @@ - (BOOL) changeFileAttributes: (NSDictionary*)attributes atPath: (NSString*)path str = [NSString stringWithFormat: @"Unable to change NSFileGroupOwnerAccountName to '%@' - %@", str, [NSError _last]]; - ASSIGN(_lastError, str); + [self _setLastError: str]; } } #endif /* _WIN32 */ @@ -559,7 +573,7 @@ - (BOOL) changeFileAttributes: (NSDictionary*)attributes atPath: (NSString*)path str = [NSString stringWithFormat: @"Unable to change NSFilePosixPermissions to '%o' - %@", (unsigned)num, [NSError _last]]; - ASSIGN(_lastError, str); + [self _setLastError: str]; } } @@ -629,7 +643,7 @@ - (BOOL) changeFileAttributes: (NSDictionary*)attributes atPath: (NSString*)path str = [NSString stringWithFormat: @"Unable to change NSFileCreationDate to '%@' - %@", date, [NSError _last]]; - ASSIGN(_lastError, str); + [self _setLastError: str]; } } @@ -678,7 +692,7 @@ - (BOOL) changeFileAttributes: (NSDictionary*)attributes atPath: (NSString*)path str = [NSString stringWithFormat: @"Unable to change NSFileModificationDate to '%@' - %@", date, [NSError _last]]; - ASSIGN(_lastError, str); + [self _setLastError: str]; } } @@ -805,7 +819,7 @@ - (NSArray*) contentsOfDirectoryAtURL: (NSURL*)url NSDirectoryEnumerator *direnum; NSString *path; - DESTROY(_lastError); + [self _setLastError: nil]; if (![[url scheme] isEqualToString: @"file"]) { @@ -908,8 +922,8 @@ - (NSDirectoryEnumerator *)enumeratorAtURL: (NSURL *)url NSString *path; BOOL shouldRecurse; BOOL shouldSkipHidden; - - DESTROY(_lastError); + + [self _setLastError: nil]; if (![[url scheme] isEqualToString: @"file"]) { @@ -953,7 +967,8 @@ - (NSArray*) contentsOfDirectoryAtPath: (NSString*)path error: (NSError**)error { NSArray *result; - DESTROY(_lastError); + [self _setLastError: nil]; + result = [self directoryContentsAtPath: path]; if (error != NULL) @@ -982,7 +997,8 @@ - (BOOL) createDirectoryAtPath: (NSString *)path { BOOL result = NO; - DESTROY(_lastError); + [self _setLastError: nil]; + if (YES == flag) { NSEnumerator *paths = [[path pathComponents] objectEnumerator]; @@ -1013,8 +1029,8 @@ - (BOOL) createDirectoryAtPath: (NSString *)path } else { - result = NO; - ASSIGN(_lastError, @"Could not create directory - intermediate path did not exist or was not a directory"); + result = NO; + [self _setLastError: @"Could not create directory - intermediate path did not exist or was not a directory"]; } } @@ -1062,7 +1078,7 @@ - (BOOL) createDirectoryAtPath: (NSString*)path /* This is consistent with MacOSX - just return NO for an invalid path. */ if ([path length] == 0) { - ASSIGN(_lastError, @"no path given"); + [self _setLastError: @"no path given"]; return NO; } @@ -1080,7 +1096,7 @@ - (BOOL) createDirectoryAtPath: (NSString*)path e = [NSString stringWithFormat: @"path %@ exists ... cannot create", path]; } - ASSIGN(_lastError, e); + [self _setLastError: e]; return NO; } else @@ -1125,7 +1141,7 @@ - (BOOL) createDirectoryAtPath: (NSString*)path e = [NSString stringWithFormat: @"Could not create '%@' - '%@'", path, [NSError _last]]; - ASSIGN(_lastError, e); + [self _setLastError: e]; return NO; } } @@ -1156,7 +1172,7 @@ - (BOOL) createFileAtPath: (NSString*)path /* This is consistent with MacOSX - just return NO for an invalid path. */ if ([path length] == 0) { - ASSIGN(_lastError, @"no path given"); + [self _setLastError: @"no path given"]; return NO; } @@ -1322,8 +1338,7 @@ - (BOOL) copyPath: (NSString*)source if ([[destination stringByAppendingString: @"/"] hasPrefix: [source stringByAppendingString: @"/"]]) { - ASSIGN(_lastError, - @"Could not copy - destination is a descendant of source"); + [self _setLastError: @"Could not copy - destination is a descendant of source"]; return NO; } @@ -1332,7 +1347,7 @@ - (BOOL) copyPath: (NSString*)source if ([self createDirectoryAtPath: destination attributes: attrs] == NO) { return [self _proceedAccordingToHandler: handler - forError: _lastError + forError: [self _lastError] inPath: destination fromPath: source toPath: destination]; @@ -1385,7 +1400,8 @@ - (BOOL) copyItemAtPath: (NSString*)src { BOOL result; - DESTROY(_lastError); + [self _setLastError: nil]; + result = [self copyPath: src toPath: dst handler: nil]; if (error != NULL) @@ -1455,7 +1471,7 @@ - (BOOL) movePath: (NSString*)source if (sourceIsDir && [[destination stringByAppendingString: @"/"] hasPrefix: [source stringByAppendingString: @"/"]]) { - ASSIGN(_lastError, @"Could not move - destination is a descendant of source"); + [self _setLastError: @"Could not move - destination is a descendant of source"]; return NO; } @@ -1499,7 +1515,8 @@ - (BOOL) moveItemAtPath: (NSString*)src { BOOL result; - DESTROY(_lastError); + [self _setLastError: nil]; + result = [self movePath: src toPath: dst handler: nil]; if (error != NULL) @@ -1570,14 +1587,14 @@ - (BOOL) linkPath: (NSString*)source if ([[destination stringByAppendingString: @"/"] hasPrefix: [source stringByAppendingString: @"/"]]) { - ASSIGN(_lastError, @"Could not link - destination is a descendant of source"); + [self _setLastError: @"Could not link - destination is a descendant of source"]; return NO; } if ([self createDirectoryAtPath: destination attributes: attrs] == NO) { return [self _proceedAccordingToHandler: handler - forError: _lastError + forError: [self _lastError] inPath: destination fromPath: source toPath: destination]; @@ -1624,7 +1641,7 @@ - (BOOL) linkPath: (NSString*)source [self changeFileAttributes: attrs atPath: destination]; return YES; #else - ASSIGN(_lastError, @"Links not supported on this platform"); + [self _setLastError: @"Links not supported on this platform"]; return NO; #endif } @@ -1646,7 +1663,7 @@ - (BOOL) removeFileAtPath: (NSString*)path lpath = [self fileSystemRepresentationWithPath: path]; if (lpath == 0 || *lpath == 0) { - ASSIGN(_lastError, @"Could not remove - no path"); + [self _setLastError: @"Could not remove - no path"]; return NO; } else @@ -1741,7 +1758,8 @@ - (BOOL) removeItemAtPath: (NSString*)path { BOOL result; - DESTROY(_lastError); + [self _setLastError: nil]; + result = [self removeFileAtPath: path handler: nil]; if (error != NULL) @@ -1767,7 +1785,8 @@ - (BOOL) createSymbolicLinkAtPath: (NSString*)path { BOOL result; - DESTROY(_lastError); + [self _setLastError: nil]; + result = [self createSymbolicLinkAtPath: path pathContent: destPath]; if (error != NULL) @@ -1797,7 +1816,7 @@ - (BOOL) fileExistsAtPath: (NSString*)path isDirectory: (BOOL*)isDirectory if (lpath == 0 || *lpath == _NUL) { - ASSIGN(_lastError, @"no path given"); + [self _setLastError: @"no path given"]; return NO; } @@ -1875,7 +1894,7 @@ - (BOOL) isReadableFileAtPath: (NSString*)path if (lpath == 0 || *lpath == _NUL) { - ASSIGN(_lastError, @"no path given"); + [self _setLastError: @"no path given"]; return NO; } @@ -1932,7 +1951,7 @@ - (BOOL) isWritableFileAtPath: (NSString*)path if (lpath == 0 || *lpath == _NUL) { - ASSIGN(_lastError, @"no path given"); + [self _setLastError: @"no path given"]; return NO; } @@ -1974,7 +1993,7 @@ - (BOOL) isExecutableFileAtPath: (NSString*)path if (lpath == 0 || *lpath == _NUL) { - ASSIGN(_lastError, @"no path given"); + [self _setLastError: @"no path given"]; return NO; } @@ -2033,7 +2052,7 @@ - (BOOL) isDeletableFileAtPath: (NSString*)path if (lpath == 0 || *lpath == _NUL) { - ASSIGN(_lastError, @"no path given"); + [self _setLastError: @"no path given"]; return NO; } @@ -2227,7 +2246,8 @@ - (NSDictionary*) attributesOfItemAtPath: (NSString*)path { NSDictionary *d; - DESTROY(_lastError); + [self _setLastError: nil]; + d = [GSAttrDictionaryClass attributesAt: path traverseLink: NO]; if (error != NULL) @@ -2368,7 +2388,7 @@ - (NSDictionary*) attributesOfFileSystemForPath: (NSString*)path return [NSDictionary dictionaryWithObjects: values forKeys: keys count: 5]; #else GSOnceMLog(@"NSFileManager", @"no support for filesystem attributes"); - ASSIGN(_lastError, @"no support for filesystem attributes"); + [self _setLastError: @"no support for filesystem attributes"]; return nil; #endif #endif /* _WIN32 */ @@ -2522,7 +2542,7 @@ - (BOOL) createSymbolicLinkAtPath: (NSString*)path return (symlink(oldpath, newpath) == 0); #else - ASSIGN(_lastError, @"symbolic links not supported on this system"); + [self _setLastError: @"symbolic links not supported on this system"]; return NO; #endif } @@ -2547,7 +2567,7 @@ - (NSString*) pathContentOfSymbolicLinkAtPath: (NSString*)path return nil; } #else - ASSIGN(_lastError, @"symbolic links not supported on this system"); + [self _setLastError: @"symbolic links not supported on this system"]; return nil; #endif } @@ -3334,7 +3354,7 @@ - (BOOL) _copyPath: (NSString*)source if (dirOK == NO) { if (![self _proceedAccordingToHandler: handler - forError: _lastError + forError: [self _lastError] inPath: destinationFile fromPath: sourceFile toPath: destinationFile]) @@ -3398,7 +3418,7 @@ - (BOOL) _copyPath: (NSString*)source s = [NSString stringWithFormat: @"cannot copy file type '%@'", fileType]; - ASSIGN(_lastError, s); + [self _setLastError: s]; NSDebugLog(@"%@: %@", sourceFile, s); continue; } @@ -3441,7 +3461,7 @@ - (BOOL) _linkPath: (NSString*)source attributes: attributes] == NO) { if ([self _proceedAccordingToHandler: handler - forError: _lastError + forError: [self _lastError] inPath: destinationFile fromPath: sourceFile toPath: destinationFile] == NO) @@ -3502,7 +3522,7 @@ - (BOOL) _linkPath: (NSString*)source LEAVE_POOL return result; #else - ASSIGN(_lastError, @"Links not supported on this platform"); + [self _setLastError: @"Links not supported on this platform"]; return NO; #endif } @@ -3552,6 +3572,28 @@ - (BOOL) _proceedAccordingToHandler: (id) handler return NO; } +- (NSString*) _lastError +{ +#if __has_feature(objc_arc) + return (__bridge NSString*)GS_THREAD_KEY_GET(thread_last_error_key); +#else + return (NSString*)GS_THREAD_KEY_GET(thread_last_error_key); +#endif +} + +- (void) _setLastError: (NSString*)error +{ +#if __has_feature(objc_arc) + NSString *oldError = (__bridge_transfer NSString*)GS_THREAD_KEY_GET(thread_last_error_key); + GS_THREAD_KEY_SET(thread_last_error_key, (__bridge_retained void*)error); +#pragma unused(oldError) +#else + NSString *oldError = (NSString*)GS_THREAD_KEY_GET(thread_last_error_key); + GS_THREAD_KEY_SET(thread_last_error_key, RETAIN(error)); + RELEASE(oldError); +#endif +} + - (NSError*) _errorFrom: (NSString *)fromPath to: (NSString *)toPath { NSError *error; @@ -3559,10 +3601,11 @@ - (NSError*) _errorFrom: (NSString *)fromPath to: (NSString *)toPath NSString *message; NSString *domain; NSInteger code; + NSString *lastError = [self _lastError]; - if (_lastError) + if (lastError) { - message = _lastError; + message = lastError; domain = NSCocoaErrorDomain; code = 0; } @@ -3599,7 +3642,12 @@ - (NSError*) _errorFrom: (NSString *)fromPath to: (NSString *)toPath error = [NSError errorWithDomain: domain code: code userInfo: errorInfo]; - DESTROY(_lastError); + + if (lastError) + { + [self _setLastError: nil]; + } + return error; }