Multithreading

[Wim Oudshoorn]

Richard Frith-Macdonald <richard@tiptree.demon.co.uk> writes:

> 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.

I think it might also depend on the alignment of the data.
If I remember correctly, but this is from the old days, aligning
a 32 bit word on a non 32 bit address would lead to a non atomic write.

But how it is transported over the memory bus is in the end quite irrelevant,
it depends on how a multiprocessor machine handles memory consistency.
(hm, another avenue to think about when confronted to multithreading bugs.)

> 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.

No, but I think that even in Objective-C the following might go wrong

   ...
   [theLock lock];
   if (theInstance == nil)
     {
       id tmp = [C new];
       [tmp setSomething: @"an argument"];
       theInstance = tmp;
     }
   [theLock unlock];
   ...

But I don't think this is a real problem because I expect the compiler
to know that assignment to a variable with a wider scope than the 'tmp' variable
is special and should not be optimized away.   But I might be wrong.

> 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;
> }
>

I think this should work.  But you could drop the isCreated == 0 clause
from the second 'if'.  Because locks should ensure memory consistency and
that means that after obtaining the lock, either 'theInstance' is 0 or correct.

Now I will come back to something you mentioned in a previous e-mail:

-[NSObject release] uses NSDecrementExtraRefCountWasZero before deallocating,
leaving a gap between decrementing and testing the refcount and calling dealloc.

Now this will create a problem, as such that the well known trick of 
writing your accessors as:

- (id) getLatestError
{
  return AUTORELEASE (RETAIN (_errorMessage));
}

- (void) setLatestError: (NSString*) err
{ 
   ASSIGN (_errorMessage, err);
}

is NOT thread safe, contrary to what is normally claimed.
(If thread A gets the error and increments the ref count just after 
thread B has decided it will dealocate it.)

There are a few ways out of this:

1 - Ignore the problem.  Because it is quite unlikely to happen
2 - Unlearn the AUTORELEASE (RETAIN ()) trick and put locks
    around all getters and setters which need to be thread safe.
3 - Change the DecRef and IncRef count is such a way that
    the refcount will be -1 just before deallocating
    and let retain return nil if the refcount is -1.
4 - the same as above, but let RETAIN throw an exception.

Any thoughts?

Wim Oudshoorn.