views:

451

answers:

8

We have a method in the iPhone SDK that is a delegate method. Problem is, the OS calls this method twice at the exact same time. This method does some heavy lifting so I don't want to execute the logic twice. What is a good way to detect this and prevent one of the two from running?


Forgot to mention that, it is called from different threads.

+6  A: 

One method is a BOOL member that you set when entering the method and clear on leaving it. If the variable is set upon entry, you know it's already executing and can just return.

Assuming you're being called from multiple threads, you'll want to lock access to this critical area of checking/setting. An NSLock is good for this.

The code below has two implementations: myMethod1 which uses NSLock and myMethod2 which shows using @synchronize.

@interface MyClass : NSObject
{
    NSLock* theLock;
    BOOL isRunning;
}
@end

@implementation MyClass

-(id)init
{
    self = [super init];
    if(self != nil)
    {
        theLock = [[NSLock alloc] init];
        isRunning = NO;
    }
    return self;
}

-(void)dealloc
{
    [theLock release];
    [super dealloc];
}

// Use NSLock to guard the critical areas
-(void)myMethod1
{
    [theLock lock];

    if(isRunning == YES)
    {
        [theLock unlock]; // Unlock before returning
        return;
    }

    isRunning = YES;        

    // Do fun stuff here

    isRunning = NO;

    [theLock unlock];    
}

// This method uses @synchronize
-(void)myMethod2
{
    @synchronized(self)
    {
        if(isRunning == YES)
        {
            return;
        }

        isRunning = YES;

        // Do stuff here.

        isRunning = NO;
    }
}
@end
nall
Any sample code?
erotsppa
It's a little more complicated than it needs to be; if you only want to run the code once you can simply do a 'if it has not run before, do stuff, otherwise unlock and return' check after locking.
Malaxeur
Thanks, Malaxeur. I tightened it up.
nall
For kicks I added a synchronize version too -- you should have plenty to choose from now!
nall
This is about 2 orders of magnitude more complex than need be. There is no need for an isRunning flag or anything like it. Just use @synchronized().
bbum
My understanding was that if one thread was executing this method, the other should return. I don't know how to do that without an isRunning (please let me know if you do). If you just need to lock access to the method and both threads can run it in its entirety, I agree a @synchronized is far simpler and the way to go.
nall
+2  A: 

simplest is to set a flag.

- (void)action {
  if (flag_is_set == NO) {
    flag_is_set = YES;
    do stuff
    flag_is_set = NO;
  }
}

this is not 100% safe though as you may get some rare cases of interlocking.

If you can handle some sleeps on the thread, use a nslock

- (id)init {
  theLock = [[NSLock alloc] init];
}
- (void)action {
  [theLock lock];
  doStuff
  [theLock unlock];
}

When thread 2 comes to the lock call and thread 1 already has it, it will sit in the execution loop until the lock is released, then it will start again. If you have UI on this thread, you app will appear to freeze

coneybeare
Any sample code?
erotsppa
i updated the code above to show how to use a lock
coneybeare
A: 

If these calls are synchronized, so only one happens at a time, you can just have a variable on your class, called something like "initialized", which starts off false and is set when initialized:

if (!initialized) {
    // handle delegated call
    initialized = 1;
}

If the delegate is called from multiple threads, you need to put all this in a mutex block.

Warren Young
It is from multiple threads, any sample code?
erotsppa
http://alienryderflex.com/NSLock.html
Warren Young
A: 

If they are called at the exact same time, I guess they are called in threads?

What you can do is define a BOOL @property (called isRunning for example) with the attribute atomic (set by default). This way this property can be accessed safely from different threads, and then you can do something like:

if (isRunning)
    return ;
isRunning = YES;
// ...
// Your code here
// ...
usRunning = NO;

You might also make sure that you are doing the right thing. If your method is called twice, maybe you're doing something wrong (or maybe it's normal, I don't know what you are doing ;))

naixn
This is tricky here. This will lock the individual setter/getter. As the apple docs point out: "It is important to understand that the goal of the atomic implementation is to provide robust accessors—it does not guarantee correctness of your code. Although “atomic” means that access to the property is thread-safe, simply making all the properties in your class atomic does not mean that your class or more generally your object graph is “thread safe”—thread safety cannot be expressed at the level of individual accessor methods. For more about multi-threading, see Threading Programming Guide."
coneybeare
Meaning, that both threads could get to line 3 because they both read isRunning atomically as NO, then both threads set isRunning to YES when they come back around on the runLoop. The only way to do it right is to lock the entire critical section, or accept that this is ok 99% of the time. I marked you down 1 because this post is claiming that atomic == threadsafe, but this only applies to the setters and getters, and for the purpose of this function, it is not threadsafe
coneybeare
You're correct. And worse, I knew it. I guess just answered too fast... :)
naixn
You are completely right (and the -1 is justified). I made the an assumption that I know was false – I just didn't realize when I wrote it though. Oh well, guess I should ponder my replies in the future, and actually double check what I'm saying :)
naixn
A: 

Here's how you can use objective-c locks as mutexes.

http://rosettacode.org/wiki/Mutex#Objective-C

Mutexes exist to allow mutually exclusive access to a certain block of code. Within the method you can do something like:

[methodLock lock]; //Blocks and waits until the lock is released
//...do stuff
[methodLock unlock]; //Releases the lock once the code has executed.

This will ensure that only one thread will be allowed within the //do stuff block of code.

EDIT: I read the question again; within the lock I'd check the flag to see if it's run (using a BOOL)

Malaxeur
+2  A: 

Some of the given answers are acceptable solutions to the problem of multiple "producer" threads calling the same function at the same time but you might be better off figuring out why multiple threads are calling this same block of code at the same time. It could be that you are assigning this delegate to multiple event handlers or something like that. You have probably noticed that this is occurring because some shared state is being mangled or the output of the function is not correct for the "global" state at the end of the function. Putting a bandaid over the fact 2 threads are in a given function (when its clear that threading was not a primary concern when this was written) is not going to necessarily give you the right results. Threading is not trivial and shared state makes it very tricky to get right, make sure that you completely understand why this is occurring before just trying to patch over it.

That being said, if you do take the bandaid approach, its probably better to do one with a lock where every thread eventually gets to execute the function rather than having them bail out if the function is allready started because to the client code it would look like that long and "heavy-lifting" process has completed and they may check for the results.

Sam Hendley
A: 

Use pthread_once() -- it was explicitly designed to do exactly this. The only problem is that the function you pass it can't take any arguments, so you have to use global variables to pass information to it.

Adam Rosenfield
Why the downvote?
Adam Rosenfield
+4  A: 

Wow. That answer is correct, but way over-engineered. Just use @synchronize().

Foo.h:

@interface Foo
{
    id expensiveResult;
}
- (void) bar;
@end

Foo.m:

@implementation Foo
- (void) bar
{
    @synchronize(self) {
        if (expensiveResult) return expensiveResult;
        .... do expensive stuff here ....
        expensiveResult = [theResult retain];
    }
    return expensiveResult;
}
@end

If you have multiple instances of Foo and want to guarantee exclusivity across all instances, create a global variable in +(void)initialize -- an NSString will do fine -- and @synchronize() on that.

However, your question raises a much more important question. In particular, there is never a case where the same method is going to be called twice simultaneously unless you quite explicitly configured your application to cause exactly that to happen.

The answer(s) provided sound more like a fix to a symptom and not a fix for the real problem.

Note: This is relying on expensiveResult being nil, which it will be as all iVars are nil on instantiation. Obviously, reset the ivar to nil if you want to recalculate.

bbum