[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Another multithreading bug (I think)

From: Richard Frith-Macdonald
Subject: Re: Another multithreading bug (I think)
Date: Fri, 8 Sep 2006 11:47:55 +0100

On 8 Sep 2006, at 08:57, David Ayers wrote:

Richard Frith-Macdonald schrieb:

Can you see any problems with this?
I don't think -retain needs to lock the gate, and as long as we are
only locking in the -release method as above, I think we can avoid
using a recursive lock (as I don't think we call -release inside any
region where we already lock the gate), but I may have missed something.

This is just tangential, but I just want to make everyone aware of the
fact that stores may not be atomic.  I'm not certain how relevant that
is for pointers/integers on IA-32 SMP platforms but for AMD64/EMT64/ IA64
SMP this may become a serious issue.

This little detail is what makes the usage of the double checked locking
idiom an issue for us also:

(The issue is described in Java but is valid for plain C just as well.)

Note that the article implies that some C++ compilers make volatile
read/stores atomic operations, but I'm certain that's not the case for
GCC (at least not for current versions).  Effectively, I believe,
volatile only means that the memory won't be cached in registers.


I understand that sacrificing double checked locking could have a harsh
performance impact, but I don't have any numbers.  Personally I would
rather sacrifice performance for correctness but I could live with a
compile time option if numbers show that the cost is very high.

While storage of a pointer may not be atomic on some architectures, it's not a problem on any modern architectures AFAIK (CPUs with 64bit pointers tend to use a 64bit memory bus and therefore do atomic reads/ write). You may know different.

However, in one of the links from wikipedia I found something else that may be a problem ... the optimisation done by the compiler may re-order assignment, so even though we have coded things to create a shared object and then assign it to a static variable, the compiler could assign to the variable before the object is fully created. This is really an issue for C++ though, not one for Objective-C, as we create instances with method calls ... effectively function calls, so the return value from the function call is assigned to the static variable and there seems to be no scope for the compiler to reassign statements to assign to it during the object creation process.

I'm guessing that storing a single byte must be atomic on all architectures, so how about this as a possible safe implementation?

- (id) sharedInstance
static volatile uint8_t isCreated = 0; // single byte declared volatile so we read from memory rather than a register
  static volatile id theInstance = nil;

  if (isCreated == 0)
      [theLock lock];   
      if (isCreated == 0 && theInstance == nil)
          theInstance = [theClass new];
      [theLock unlock];
      isCreated = 1;
  return theInstance;

Because the assignment to isCreated occurs outside the locked region, we can be sure the compiler won't re-order it before the assignment to theInstance, so this implementation is safe from the issue of compiler re-ordering statements (if it applies to Objective-C code ... it probably doesn't, but may become an issue with Objective-C ++) as well as any worries about pointer assignments not being atomic.

To the best of my knowledge, the use of a lock should force ordering even in the case of a multiprocessor system where each processor has its own local cache ... as I think the lock implementation ought to ensure cache consistency.

Since I currently don't have the time to deal with this, I can only
offer writing up a bug report.  It would be great if someone had the
time to investigate how others have implemented test cases for
multi-threaded issues and put a pointer on the wiki.

I've never really seen testcases for threading issues ...

reply via email to

[Prev in Thread] Current Thread [Next in Thread]