dispatch_semaphore_t reuse - What am I missing here?

Question

I have some code where I am using dispatch_semaphore_t to signal operation completion. When the semaphore is a member variable, it does not seem to behave correctly. I will show example code that works and an example that does not seem to work:

@implementation someClass  
{  
  dispatch_semaphore_t memberSem;  
  dispatch_semaphore_t* semPtr;  
  NSThread* worker;
  BOOL taskDone;  
}  

- (id)init  
{  
  // Set up the worker thread and launch it - not shown here.  
  memberSem= dispatch_semaphore_create(0); 
  semPtr= NULL;  
  taskDone= FALSE;  
}  

- (void)dealloc  
{  
  // Clean up the worker thread as needed - not shown here.  
  if((NULL != semPtr) && (NULL != *semPtr))  
    disptatch_release(*semPtr);  

  dispatch_release(memberSem);  
}  

- (void)doSomethingArduous  
{  
  while([self notDone])  // Does something like check a limit.  
    [self doIt];  // Does something like process data and increment a counter.  

  taskDone= TRUE;  // I know this should be protected, but keeping the example simple for now.  

  if((NULL != semPtr) && (NULL != *semPtr))  
    dispatch_semaphore_signal(*semPtr);  // I will put a breakpoint here, call it  "SIGNAL"  
}  

- (BOOL)getSomethingDoneUseLocalSemaphore  
{  
  taskDone= FALSE;  // I know this should be protected, but keeping the example simple for now.  
  dispatch_semaphore_t localSem= dispatch_semaphore_create(0);  
  semPtr= &localSem;  
  [self performSelector:doSomethingArduous onThread:worker withObject:nil waitUntilDone:NO];  

  dispatch_time_t timeUp= dispatch_time(DISPATCH_TIME_NOW, (uint64_t)(2.5 * NSEC_PER_SEC));  
  dispatch_semaphore_wait(localSem, timeUp);  

  semPtr= NULL;  
  dispatch_release(localSem);  

  // I know I could just return taskDone. The example is this way to show what the problem is.  
  if(taskDone)  // Again with thread safety.  
    return TRUE;    

  return FALSE;  
}  

- (BOOL)getSomethingDoneUseMemberSemaphore  
{  
  taskDone= FALSE;  // I know this should be protected, but keeping the example simple for now.  

  semPtr= &memberSem;  // I will put a breakpoint here, call it "START"  
  [self performSelector:doSomethingArduous onThread:worker withObject:nil waitUntilDone:NO];  

  dispatch_time_t timeUp= dispatch_time(DISPATCH_TIME_NOW, (uint64_t)(2.5 * NSEC_PER_SEC));  
  dispatch_semaphore_wait(memberSem, timeUp);  

  semPtr= NULL;  

  // I know I could just return taskDone. The example is this way to show what the problem is.  
  if(taskDone)  // Again with thread safety.  
    return TRUE;  // I will put a breakpoint here, call it "TASK_DONE"   

  return FALSE;  // I will put a breakpoint here, call it "TASK_NOT_DONE"  
}  

- (void)hereIsWhereWeBringItTogether  
{  
  BOOL gotItDoneLocal= [self getSomethingDoneUseLocalSemaphore];  // Will return TRUE.  
  gotItDoneLocal= [self getSomethingDoneUseLocalSemaphore];  // Will return TRUE.  
  gotItDoneLocal= [self getSomethingDoneUseLocalSemaphore];  // Will return TRUE.  

  BOOL gotItDoneMember= [self getSomethingDoneUseMemberSemaphore];  // Will return TRUE. I will put a breakpoint here, call it "RUN_TEST"  
  gotItDoneMember= [self getSomethingDoneUseMemberSemaphore];  // Will return FALSE.  
}  

So, given that code and the results I get/got, I put the breakpoints as described in my real code: One in the main function, one to start in the work function, one where the member semaphore is signaled, and two after the wait.

What I found was in the case where I use the member semaphore, in the first round I stop at breakpoint "RUN_TEST", run and hit breakpoint "START", run then hit breakpoint "SIGNAL", run then hit breakpoint "TASK_DONE" - all as expected.

When I continue to run, I hit breakpoint "START", run then hit breakpoint "TASK_NOT_DONE", run then hit breakpoint "SIGNAL"

That is, when I run the sequence using a semaphore that is a member, and do what looks like proper signal/wait, the second time I try to wait on that semaphore I seem to blow by and it gets signaled after I have exited the wait.

I seem to either be not managing the counting right (signal/wait pairings) or that member semaphore will not go back to an un-signaled state.

My feeling is there is something fundamental I am missing here. Any input would be appreciated.

EDIT: Ultimately what I seemed to be missing was due to my actual code being a bit more complicated. Instead of a clean return from the arduous task, there are multiple threads involved and a postNotification. I replaced the postNotification with the code in the notification handler - it sets a flag and signals the semaphore. That way any delay that might have been introduced by the notification handler is eliminated.

Solution 1

Yes, this is the expected behavior. If you time out waiting for a signal, when the signal comes it, it will be caught by the next call to dispatch_semaphore_wait for that particular semaphore. Consider the following example:

For example:

dispatch_semaphore_t semaphore = dispatch_semaphore_create(0);
dispatch_time_t timeout;

// in 5 seconds, issue signal

dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
    sleep(5);
    NSLog(@"Signal 1");
    dispatch_semaphore_signal(semaphore);
});

// wait four seconds for signal (i.e. we're going to time out before the signal)

NSLog(@"Waiting 1");
timeout = dispatch_time(DISPATCH_TIME_NOW, (int64_t)(4.0 * NSEC_PER_SEC));
if (dispatch_semaphore_wait(semaphore, timeout))
    NSLog(@"Waiting for 1: timed out");
else
    NSLog(@"Waiting for 1: caught signal");

// now, let's issue a second signal in another five seconds

dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
    sleep(5);
    NSLog(@"Signal 2");
    dispatch_semaphore_signal(semaphore);
});

// wait another four seconds for signal

// this time we're not going to time out waiting for the second signal, 
// because we'll actually catch that first signal, "signal 1")

NSLog(@"Waiting 2");
timeout = dispatch_time(DISPATCH_TIME_NOW, (int64_t)(4.0 * NSEC_PER_SEC));
if (dispatch_semaphore_wait(semaphore, timeout))
    NSLog(@"Waiting for 2: timed out");
else
    NSLog(@"Waiting for 2: caught signal");

// note, "signal 2" is still forthcoming and the above code's 
// signals and waits are unbalanced 

So, when you use the class instance variable, your getSomethingDoneUseMemberSemaphore behaves like above, where the second call to dispatch_semaphore_wait will catch the first signal issued because (a) it's the same semaphore; and (b) if the first call to dispatch_semaphore_signal timed out.

But if you use unique semaphores each time, then the second call to dispatch_semaphore_wait will not respond to the dispatch_semaphore_signal of the first semaphore.

Solution 2

When you call dispatch_semaphore_wait with a timeout, and the thread is still blocked at the timeout, what happens is almost the same as if dispatch_semaphore_signal had been called. One difference is that dispatch_semaphore_signal would wake up any thread, but the timeout wakes up this particular thread. The other difference is that dispatch_semaphore_wait will return a non-zero value instead of 0.

Here's the problem: Whoever was going to call dispatch_semaphore_signal is still going to call it, and then we have one signal too many. This may be hard to avoid; if you have a 10 second timeout then dispatch_semaphore_signal could be called after 10.000000001 seconds. So if you are reusing the semaphore, you have a problem at your hand.

On the other hand, if you are not reusing the semaphore then the worst that happens is that the semaphore count goes to 1. But that's no problem.

Summary: Don't reuse a semaphore if you wait for it with a timeout.

Solution 3

I was able to code up something akin to what I think you're looking for and it appears to work the way you want it to (but again, I'm not 100% sure I understand what you're looking for.):

ArduousTaskDoer.m

@implementation ArduousTaskDoer
{
    dispatch_semaphore_t mSemaphore;
    BOOL mWorkInProgress;
}

- (id)init
{
    if (self = [super init])
    {
        mSemaphore = dispatch_semaphore_create(0);
    }
    return self;
}

- (void)dealloc
{
    mSemaphore = nil;
}

- (void)doWork
{
    @synchronized(self)
    {
        mWorkInProgress = YES;
    }

    // Do stuff
    sleep(10);

    @synchronized(self)
    {
        mWorkInProgress = NO;
    }

    dispatch_semaphore_signal(mSemaphore);
}

- (BOOL)workIsDone
{

    @synchronized(self)
    {
        if (!mWorkInProgress)
        {
            mWorkInProgress = YES;
            dispatch_async(dispatch_get_global_queue(0, 0), ^{
                [self doWork];
            });
        }
    }


    if (dispatch_semaphore_wait(mSemaphore, dispatch_time(DISPATCH_TIME_NOW, (int64_t)2.5 * NSEC_PER_SEC)))
    {
        return NO;
    }

    return YES;
}

@end

...and then the calling code:

ArduousTaskDoer* task = [[ArduousTaskDoer alloc] init];
BOOL isDone = NO;
while(!(isDone = [task workIsDone]))
{
    NSLog(@"Work not done");
}

NSLog(@"Work is done");

// Do it again... Semaphore is being reused
while(!(isDone = [task workIsDone]))
{
    NSLog(@"Work not done");
}

NSLog(@"Work is done");

Hope this helps.

Comments

[GTAE86]

I see what you mean. Basically, if the task takes longer than the wait, the wait will timeout, but the signal will still increment the counter, so the next wait will fire immediately. That was something I thought about, but in this case I can hook up an analyzer and see that the action completes in sub-second time. I guess I could test whether the semaphore is signaled the second time by just doing a wait before I start my long process. If it does not timeout, I know what is going on and can adjust my wait time appropriately.

[GTAE86]

@impcc - Thanks for the answer. Sorry, but I am confused - my debugger reports that dispatch_time(DISPATCH_TIME_NOW, (int64_t)(2.5 * NSEC_PER_SEC)) returns the same value as dispatch_time(DISPATCH_TIME_NOW, (int64_t)2.5 * NSEC_PER_SEC). Am I missing something? As far as your understanding of what I am trying to do - you have it right. I am just trying to wait in one thread on a task to complete in another thread. In the case of your example, it looks like you are waiting 2.5 seconds for a 10-second task to complete, so the signal will occur after the wait has elapsed. Correct?

[Rob]

@GTAE86 Correct. BTW, the way you have your local semaphore code set up, you could, theoretically still see the same behavior as your member variable example (it's entirely a matter of the precise timing). If you absolutely don't want cross-talk between the various signals of various tasks, rather than picking up the semaphore from some instance variable (even your current "local variable" example ends up storing the semaphore in an instance variable), you want to pass the semaphore as a parameter, not using an instance variable (or make a separate instance of your object for each operation).

[ipmcc]

I see... I misread your code and I thought you were just passing (2.5 * NSEC_PER_SEC) directly to dispatch_semaphore_wait. My mistake. Yes, the signal will occur after the first wait. If you call the waiting function more than once it will wait 2.5 seconds on each call. If you just want to wait until the task is fully done, just use DISPATCH_TIME_FOREVER. Alternately, if you want to be notified asynchronously of the completion of the task, why not use dispatch_group_notify?

[GTAE86]

I think I may see what is going on in my code. Multiple threads are in play (way more complicated than example). Arduous process is: In thread A call thread B to write to a device, wait. Thread C gets response (ReadPipeAsync), caches it in a deque. Thread D plucks it off for analysis. If good, it posts a notification with the result. Notification handler does some checking, sets the completion status (to a lock-protected variable) and signals the semaphore if requested. Though I know my operation completed quickly, that notification might be delayed.

[GTAE86]

I see what you mean about the variable identity. I was actually asking this question because I was hoping to avoid sending the semaphore along in a selector! That is mainly due to logistics in the code, and those issues can likely be overcome. I was not certain of how to pass the semaphore - can it just be shoved into performSelector:onThread:withObject:waitUntilDone: as the object?

[Rob]

@GTAE86 The GCD calls in my example end up on separate worker threads, too (it's a much easier way to do threads), but I get your point. Sounds like you've nailed, though.

[GTAE86]

Mainly because I am not really using GCD, just the semaphores. From what I have read, I might be breaking all sorts of "rules" by doing it, but it seems to work very well. I have read all of the Apple guides on concurrency and threading, and know they recommend against threads if possible. My case is I have to fire up open-ended I/O sessions with multiple chips - basically tell the chips to start gathering data and process it until I tell them to stop. Real-world users might realistically run it for hours at a time, so the command/control and data collection threads can run a long time.

[Rob]

@GTAE86 Whether you can pass a semaphore as an object might depend upon whether your minimum iOS target is 6.0 or whether it's an earlier version. (Effective iOS 6, GCD objects are objects.) I might be inclined to make an custom class with a single property, a dispatch_semaphore_t, and pass that object.

[GTAE86]

I thought about that. For now, though, my app is OS X only, and I am targeting 10.8 with the API that does all the work.

[GTAE86]

Looks like I will have to wrap it after all - the compiler says dispatch_semaphore_t is "struct dispatch_semaphore_s*"

[GTAE86]

Right. I ended up creating a semaphore-holder class, and when I launch an operation I create an instance of it, set up the semaphore, and pop it into a class-owned thread-safe collection. That way the completion code running in another thread can check for it and signal it if it exists. The main problem I was having in the case above was that I was signalling the semaphore in a notification handler: That introduced a delay that caused me to time out in the wait. So, there were two things I was doing that led to the problem. Thanks for the input!