Fix bug setting name of an image which already has a name. Added thread safety

git-svn-id: svn+ssh://svn.gna.org/svn/gnustep/libs/gui/trunk@28743 72102866-910b-0410-8b05-ffd578937521

ChangeLog

@@ -1,3 +1,11 @@
+2009-09-27 Richard Frith-Macdonald <rfm@gnu.org>
+
+	* Source/NSImage.m: ([-setName:]) fix bug which was removing the	
+	wrong name when given an image which already had a name.
+	Make named images thread-safe using a lock.
+	Add private method to update names on theme change.
+	* Source/GSTheme.m: Use new method to update theme images.
+	
 2009-09-26 Fred Kiefer <FredKiefer@gmx.de>
 	
 	* Source/NSApplication.m (-setApplicationIconImage:,

Source/GSTheme.m

@@ -78,6 +78,10 @@ NSString	*GSThemeWillActivateNotification
 NSString	*GSThemeWillDeactivateNotification
   = @"GSThemeWillDeactivateNotification";
 
+@interface	NSImage (GSTheme)
++ (NSImage*) _setImage: (NSImage*)image name: (NSString*)name;
+@end
+
 @interface	GSTheme (Private)
 - (void) _revokeOwnerships;
 @end
@@ -366,18 +370,7 @@ typedef	struct {
 	   */
 	  if (image != nil && [[image name] isEqualToString: imageName] == NO)
 	    {
-	      if ([image setName: imageName] == NO)
+	      [NSImage _setImage: image name: imageName];
-	        {
-		  NSImage	*old;
-
-		  /* Couldn't set image name ... presumably already
-		   * in use ... so we remove the name from the old
-		   * image and try again.
-		   */
-		  old = [NSImage imageNamed: imageName];
-		  [old setName: nil];
-	          [image setName: imageName];
-		}
 	    }
 	}
     }
@@ -584,26 +577,26 @@ typedef	struct {
     userInfo: nil];
 
   /*
-   * Remove all cached bundle images from both NSImage's name dictionary
+   * Restore old images in NSImage's lookup dictionary so that the app
+   * still has images to draw.
+   * The remove all cached bundle images from both NSImage's name dictionary
    * and our cache dictionary, so that we can be sure we reload afresh
    * when re-activated (in case the images on disk changed ... eg by a
    * theme editor modifying the theme).
-   * Restore old images in NSImage's lookup dictionary so that the app
-   * still has images to draw.
    */
+  enumerator = [_oldImages keyEnumerator];
+  while ((name = [enumerator nextObject]) != nil)
+    {
+      image = [_oldImages objectForKey: name];
+      [NSImage _setImage: image name: name];
+    }
+  [_oldImages removeAllObjects];
   enumerator = [_images objectEnumerator];
   while ((image = [enumerator nextObject]) != nil)
     {
       [image setName: nil];
     }
   [_images removeAllObjects];
-  enumerator = [_oldImages keyEnumerator];
-  while ((name = [enumerator nextObject]) != nil)
-    {
-      image = [_oldImages objectForKey: name];
-      [image setName: name];
-    }
-  [_oldImages removeAllObjects];
 
   [self _revokeOwnerships];
 

Source/NSImage.m

@@ -28,31 +28,32 @@
 #include <string.h>
 #include <math.h>
 
-#include <Foundation/NSArray.h>
+#import <Foundation/NSArray.h>
-#include <Foundation/NSBundle.h>
+#import <Foundation/NSBundle.h>
-#include <Foundation/NSDebug.h>
+#import <Foundation/NSDebug.h>
-#include <Foundation/NSDictionary.h>
+#import <Foundation/NSDictionary.h>
-#include <Foundation/NSException.h>
+#import <Foundation/NSException.h>
-#include <Foundation/NSFileManager.h>
+#import <Foundation/NSFileManager.h>
-#include <Foundation/NSKeyedArchiver.h>
+#import <Foundation/NSLock.h>
-#include <Foundation/NSString.h>
+#import <Foundation/NSKeyedArchiver.h>
-#include <Foundation/NSValue.h>
+#import <Foundation/NSString.h>
+#import <Foundation/NSValue.h>
 
-#include "AppKit/NSImage.h"
+#import "AppKit/NSImage.h"
 
-#include "AppKit/AppKitExceptions.h"
+#import "AppKit/AppKitExceptions.h"
-#include "AppKit/NSAffineTransform.h"
+#import "AppKit/NSAffineTransform.h"
-#include "AppKit/NSBitmapImageRep.h"
+#import "AppKit/NSBitmapImageRep.h"
-#include "AppKit/NSCachedImageRep.h"
+#import "AppKit/NSCachedImageRep.h"
-#include "AppKit/NSColor.h"
+#import "AppKit/NSColor.h"
-#include "AppKit/NSPasteboard.h"
+#import "AppKit/NSPasteboard.h"
-#include "AppKit/NSPrintOperation.h"
+#import "AppKit/NSPrintOperation.h"
-#include "AppKit/NSScreen.h"
+#import "AppKit/NSScreen.h"
-#include "AppKit/NSView.h"
+#import "AppKit/NSView.h"
-#include "AppKit/NSWindow.h"
+#import "AppKit/NSWindow.h"
-#include "AppKit/PSOperators.h"
+#import "AppKit/PSOperators.h"
-#include "GNUstepGUI/GSDisplayServer.h"
+#import "GNUstepGUI/GSDisplayServer.h"
-#include "GSThemePrivate.h"
+#import "GSThemePrivate.h"
 
 
 /* Helpers.  Would be nicer to use the C99 fmin/fmax functions, but that
@@ -134,9 +135,10 @@ BOOL NSImageForceCaching = NO; /* use on missmatch */
 @end
 
 /* Class variables and functions for class methods */
-static NSMutableDictionary *nameDict = nil;
+static NSRecursiveLock		*imageLock = nil;
-static NSDictionary *nsmapping = nil;
+static NSMutableDictionary	*nameDict = nil;
-static NSColor *clearColor = nil;
+static NSDictionary		*nsmapping = nil;
+static NSColor			*clearColor = nil;
 static Class cachedClass = 0;
 static Class bitmapClass = 0;
 
@@ -175,35 +177,42 @@ repd_for_rep(NSArray *_reps, NSImageRep *rep)
 
 + (void) initialize
 {
-  if (self == [NSImage class])
+  if (imageLock == nil)
     {
-      NSString *path = [NSBundle pathForLibraryResource: @"nsmapping"
+      NSString *path;
-                                                 ofType: @"strings"
+
-                                            inDirectory: @"Images"];
+      imageLock = [NSRecursiveLock new];
+      [imageLock lock];
 
       // Initial version
       [self setVersion: 1];
 
       // initialize the class variables
       nameDict = [[NSMutableDictionary alloc] initWithCapacity: 10];
+      path = [NSBundle pathForLibraryResource: @"nsmapping"
+				       ofType: @"strings"
+				  inDirectory: @"Images"];
       if (path)
         nsmapping = RETAIN([[NSString stringWithContentsOfFile: path]
                                propertyListFromStringsFileFormat]);
       clearColor = RETAIN([NSColor clearColor]);
       cachedClass = [NSCachedImageRep class];
       bitmapClass = [NSBitmapImageRep class];
+      [imageLock unlock];
     }
 }
 
 + (id) imageNamed: (NSString *)aName
 {
-  NSImage	*image = (NSImage*)[nameDict objectForKey: aName];
+  NSImage	*image;
  
   /* 2009-09-10 changed operation of nsmapping so that the loaded
    * image is stored under the key 'aName', not under the mapped
    * name.  That way the image is created with the correct name and
    * a later call to -setName: will work properly.
    */
+  [imageLock lock];
+  image = (NSImage*)[nameDict objectForKey: aName];
   if (image == nil || [(id)image _resource] == nil)
     {
       NSString	*realName = [nsmapping objectForKey: aName];
@@ -217,7 +226,6 @@ repd_for_rep(NSArray *_reps, NSImageRep *rep)
           realName = aName;
 	}
  
-
       // FIXME: This should use [NSBundle pathForImageResource], but this will 
       // only allow imageUnfilteredFileTypes.
       /* If there is no image with that name, search in the main bundle */
@@ -298,7 +306,8 @@ repd_for_rep(NSArray *_reps, NSImageRep *rep)
           image = (NSImage*)[nameDict objectForKey: aName];
         }
     }
-  
+  IF_NO_GC([[image retain] autorelease]);
+  [imageLock unlock];
   return image;
 }
 
@@ -512,9 +521,7 @@ repd_for_rep(NSArray *_reps, NSImageRep *rep)
   return copy;
 }
 
-/* This method is used by the GSTheme class to set the names of system
+/*
- * images.  It *must* be possible to unset an old system image name by
- * passing a nil value to aName.
  * The images are actually accessed via proxy objects, so that when a
  * new system image is set, the proxies for that image just start using
  * the new version.
@@ -523,10 +530,13 @@ repd_for_rep(NSArray *_reps, NSImageRep *rep)
 {
   GSThemeProxy	*proxy = nil;
   
+  [imageLock lock];
+
   /* The name is already set... nothing to do.
    */
   if (aName == _name || [aName isEqual: _name] == YES)
     {
+      [imageLock unlock];
       return YES;
     }
 
@@ -535,23 +545,25 @@ repd_for_rep(NSArray *_reps, NSImageRep *rep)
    */
   if (aName != nil && [[nameDict objectForKey: aName] _resource] != nil)
     {
+      [imageLock unlock];
       return NO;
     }
 
   /* If this image had another name, we remove it.
    */
-  if (_name && self == [(proxy = [nameDict objectForKey: _name]) _resource])
+  if (_name != nil)
     {
       /* We retain self in case removing from the dictionary releases us */
       IF_NO_GC([[self retain] autorelease]);
+      [nameDict removeObjectForKey: _name];
       DESTROY(_name);
-      [proxy _setResource: nil];
     }
   
   /* If the new name is null, there is nothing more to do.
    */
   if (aName == nil)
     {
+      [imageLock unlock];
       return NO;
     }
 
@@ -565,12 +577,18 @@ repd_for_rep(NSArray *_reps, NSImageRep *rep)
     }
   [proxy _setResource: self];
   
+  [imageLock unlock];
   return YES;
 }
 
 - (NSString *) name
 {
-  return _name;
+  NSString	*name;
+
+  [imageLock lock];
+  name = [[_name retain] autorelease];
+  [imageLock unlock];
+  return name;
 }
 
 - (void) setSize: (NSSize)aSize
@@ -2038,3 +2056,37 @@ iterate_reps_for_types(NSArray* imageReps, SEL method)
 }
 
 @end
+
+@implementation	NSImage (GSTheme)
+
++ (NSImage*) _setImage: (NSImage*)image name: (NSString*)name
+{
+  GSThemeProxy	*proxy = nil;
+  
+  NSAssert([image isKindOfClass: [NSImage class]], NSInvalidArgumentException);
+  NSAssert(![image isProxy], NSInvalidArgumentException);
+  NSAssert([name isKindOfClass: [NSString class]], NSInvalidArgumentException);
+  NSAssert([name length] > 0, NSInvalidArgumentException);
+  NSAssert([image name] == nil, NSInvalidArgumentException);
+
+  [imageLock lock];
+  ASSIGNCOPY(image->_name, name);
+  if ((proxy = [nameDict objectForKey: image->_name]) == nil)
+    {
+      proxy = [GSThemeProxy alloc];
+      [nameDict setObject: proxy forKey: image->_name];
+      [proxy release]; 
+    }
+  else
+    {
+      /* Remove the name from the old image.
+       */
+      DESTROY(((NSImage*)[proxy _resource])->_name);
+    }
+  [proxy _setResource: image];
+  IF_NO_GC([[proxy retain] autorelease]);
+  [imageLock unlock];
+  return (NSImage*)proxy;
+}
+
+@end