Re: Deadlock in NSUserDefaults

[Larry Campbell]

On Jun 6, 2008, at 6:12 PM, Larry Campbell wrote:

> I just found a deadlock bug in NSUserDefaults. I think this may be  
> the same bug as the one mbruen@smartsoft.de reported last year under  
> the subject line "NSRecursiveLock hangs".
>
> Here's a timeline. Line numbers refer to NSUserDefaults.m in gnustep- 
> base 1.13.0 (although a quick glance at 1.14.1 leads me to believe  
> it still has the same bug). All of the instance methods are being  
> called on the same instance, the default NSUserDefaults object.
>
> thread 1 calls -[NSUserDefaults synchronize], which acquires _lock  
> (line 1588) and calls -[NSUserDefaults writeDefaults]
> thread 1 -[NSUserDefaults writeDefaults] calls -[NSDictionary  
> writeToFile:atomically:]
> thread 2 calls NSLog, which calls NSCalendarDate, which calls  
> GSUserDefaultsDictionaryRepresentation
> thread 2 GSUserDefaultsDictionaryRepresentation acquires classLock  
> (line 1968)
> thread 1 -[NSDictionary writeToFile:atomically:] calls  
> GSUserDefaultsDictionaryRepresentation
> thread 1 GSUserDefaultsDictionaryRepresentation attempts to acquire  
> classLock (line 1968) and hangs because thread 2 has it
> thread 2 GSUserDefaultsDictionaryRepresentation calls - 
> [NSUserDefaults dictionaryRepresentation]
> thread 2 -[NSUserDefaults dictionaryRepresentation] attempts to  
> acquire _lock (line 1765) and hangs because thread 1 has it
>
> This is a classic lock ordering bug. One simple way to avoid it it  
> to always follow the rule that whenever multiple locks are acquired,  
> they must be acquired in the same order, and released in the reverse  
> order. However, at first glance, it looks to me like making  
> NSUserDefaults.m conform to this rule might require major surgery.

Although I think there may be other bugs lurking here, I think a  
localized fix for this one would be to change

NSDictionary*
GSUserDefaultsDictionaryRepresentation()
{
  NSDictionary  *defs;

  if (sharedDefaults == nil)
    {
      [NSUserDefaults standardUserDefaults];
    }
  [classLock lock];
  defs = [sharedDefaults dictionaryRepresentation];
  [classLock unlock];
  return defs;
}


to


NSDictionary*
GSUserDefaultsDictionaryRepresentation()
{
  NSUserDefaults *shared;

  if (sharedDefaults == nil)
    {
      [NSUserDefaults standardUserDefaults];
    }
  [classLock lock];
  shared = AUTORELEASE(RETAIN(sharedDefaults));
  [classLock unlock];
  return [shared dictionaryRepresentation];
}