-(NSArray *)people not thread-safe about rhaddressbook HOT 6 CLOSED

Thanks for the test case!

This appears to be a concurrency issue and is not specifically limited to the access occurring on an arbitrary thread.

I've had a quick look at it, however will need to do some further work before i can find a possible solution. Basically accessing people] from 2 threads concurrently is causing an overrelease via way of autorelease.

For now you can call the below to prevent it.
[self.queue setMaxConcurrentOperationCount:1];

However this is not exactly ideal and so i will try to come up with a fix.

from rhaddressbook.

Of course the problem is related to different threads accessing people at the same time - concurrency is achieved by threads even though it's GCD managing them and not me.

I don't understand how autorelease is even an issue if I'm using ARC.

Unfortunately, the operation I'm doing that raised the issue requires concurrency because it has both IO and computation, and this way I get computations done while some operations are blocking on IO. For my app, I worked around it by accessing people beforehand, caching it, and accessing the cached version from within my operations.

from rhaddressbook.

This is a pure and simple race condition caused by calling retain / release from multiple threads at once.

This is still an issue with ARC enabled because, believe it for not, ARC still uses retain and release as well as autorelease pools.

If you build RHAddressBook without using ARC, you can fix the issue by forcing release to be performed on a single addressbook thread. ie

-(oneway void)release{
    if (_addressBook.addressBookThread && ![[NSThread currentThread] isEqual:_addressBook.addressBookThread]) {
        [self performSelector:_cmd onThread:_addressBook.addressBookThread withObject:nil waitUntilDone:NO];
    } else {
        [super release];
    }
}

However if using ARC the best solution for the moment is likely to keep a copy of the people array retained until your operation queue is empty, then release the array.

RHAddressbook instances don't currently retain their vended RHRecord objects because the objects retain their addressbook instance. This allows you to get a particular RHPerson Instance and then only store the person object, forgetting about the underlying addressbook instance.

Some background:
A race condition occurs when two threads access a shared variable at the same time. The first thread reads the variable, and the second thread reads the same value from the variable. Then the first thread and second thread perform their operations on the value, and they race to see which thread can write the value last to the shared variable. The value of the thread that writes its value last is preserved, because the thread is writing over the value that the previous thread wrote.

from rhaddressbook.

Calling retain/release from multiple threads is fine - see http://developer.apple.com/library/ios/#documentation/Cocoa/Conceptual/Multithreading/ThreadSafetySummary/ThreadSafetySummary.html

I've worked with reference counting systems in the past (e.g. shared_ptr in C++) and
"Object allocation and retain count functions" is listed under the explicitly thread-safe operations.

Some reading on the internet implies that the release as part of an autorelease pool is not thread-safe (http://stackoverflow.com/questions/6512540/thread-safe-retain-release and one or two other similar posts), even though retain and release are, but this is also a bit weird, since it implies you can not transfer ownership of any object you pushed onto an autorelease pool to another thread, which means the entire library isn't thread-safe anyway since every RHPerson is created and pushed to an autorelease pool on _addressBookThread and has then ownership transferred to a different thread, and this just happens to be working because the average use-case doesn't stress the retain/release/autorelease machinery of the RHPersons thrown around. However, this seems like a way too big limitation on how to write code in Objective C, so I'm hoping I misunderstood something here. Did I?

from rhaddressbook.

You are correct in saying that retain and release are both thread safe. The use a lock internally.

You can get a good sense of how things work here. http://www.opensource.apple.com/source/CF/CF-744.12/CFRuntime.c ( look for _CFRelease )

The issue you are seeing, however is a race condition in the managment of our per RHAddressBook instance uniquing cache.

We use a uniquing cache inside of each RHAddressBook instance, and the removal from said cache is currently managed from inside dealloc. (Possibly improperly.)

The removal can currently race with other threads which are looking up and finding objects from those same caches. (ie retain hits zero, however before dealloc is called another thread asks for a given object that is currently still in the uniquing cache, albeit with a retain count of zero.)

This cache maintanance would have made sence being moved into finalize, however as far as i can tell finalize is no longer called by the runtime in non GC cases.

This is disapointing because previous versions of the class actually have a comment that is totally applicable in this current situation. (Line 1365.)

http://opensource.apple.com/source/CF/CF-550.42/CFRuntime.c

// We recheck lowBits to see if the object has been retained again during
// the finalization process.  This allows for the finalizer to resurrect,
// but the main point is to allow finalizers to be able to manage the
// removal of objects from uniquing caches, which may race with other threads
// which are allocating (looking up and finding) objects from those caches,
// which (that thread) would be the thing doing the extra retain in that case.

Will possibly file a radar.

from rhaddressbook.

098fda0

from rhaddressbook.