Fixes up a race condition in GTMNSThread+Blocks (#181)

There was a race between the thread being finished and
isFinished/isExecuting reporting correctly.
There may have also been a locking issue on older single processor phones.

3 files changed, 224 insertions, 318 deletions

diff --git a/Foundation/GTMNSThread+Blocks.h b/Foundation/GTMNSThread+Blocks.h
index 17bfbc7..c6e5ebc 100644
--- a/Foundation/GTMNSThread+Blocks.h
+++ b/Foundation/GTMNSThread+Blocks.h
@@ -38,15 +38,14 @@
 
 #endif  // NS_BLOCKS_AVAILABLE
 
-// A simple thread that does nothing but handle performBlock and
-// performSelector calls.
-@interface GTMSimpleWorkerThread : NSThread {
- @private
-  CFRunLoopRef runLoop_;
-  NSConditionLock *runLock_;
-}
-
-// Will stop the thread, blocking till the thread exits.
+// A simple thread that does nothing but runs a runloop.
+// That means that it can handle performBlock and performSelector calls.
+@interface GTMSimpleWorkerThread : NSThread
+
+// If called from another thread, blocks until worker thread is done.
+// If called from the worker thread it is equivalent to cancel and
+// returns immediately.
+// Note that "stop" will set the isCancelled on the thread.
 - (void)stop;
 
 @end
diff --git a/Foundation/GTMNSThread+Blocks.m b/Foundation/GTMNSThread+Blocks.m
index 8318193..3ef4a45 100644
--- a/Foundation/GTMNSThread+Blocks.m
+++ b/Foundation/GTMNSThread+Blocks.m
@@ -53,220 +53,62 @@
 
 #endif  // NS_BLOCKS_AVAILABLE
 
-enum {
-  kGTMSimpleThreadInitialized = 0,
-  kGTMSimpleThreadStarting,
-  kGTMSimpleThreadRunning,
-  kGTMSimpleThreadCancel,
-  kGTMSimpleThreadFinished,
-};
-
 @implementation GTMSimpleWorkerThread
 
-- (id)init {
-  if ((self = [super init])) {
-    runLock_ =
-        [[NSConditionLock alloc] initWithCondition:kGTMSimpleThreadInitialized];
-  }
-  return self;
-}
-
-- (void)dealloc {
-  if ([self isExecuting]) {
-    [self stop];
-  }
-  [runLock_ release];
-  [super dealloc];
-}
-
-- (void)setThreadDebuggerName:(NSString *)name {
-  if ([name length]) {
-    pthread_setname_np([name UTF8String]);
-  } else {
-    pthread_setname_np("");
-  }
-}
-
 - (void)main {
-  [runLock_ lock];
-  if ([runLock_ condition] != kGTMSimpleThreadStarting) {
-    // Don't start, we're already cancelled or we've been started twice.
-    [runLock_ unlock];
-    return;
-  }
-
-  // Give ourself an autopool
-  NSAutoreleasePool *localPool = [[NSAutoreleasePool alloc] init];
-
-  // Expose the current runloop so other threads can stop (but see caveat
-  // below).
-  NSRunLoop *loop = [NSRunLoop currentRunLoop];
-  runLoop_ = [loop getCFRunLoop];
-  if (runLoop_) CFRetain(runLoop_);  // NULL check is pure paranoia.
-
-  // Add a port to the runloop so that it stays alive. Without a port attached
-  // to it, a runloop will immediately return when you call run on it.
-  [loop addPort:[NSPort port] forMode:NSDefaultRunLoopMode];
-
-  // Name ourself
-  [self setThreadDebuggerName:[self name]];
-
-  // We're officially running.
-  [runLock_ unlockWithCondition:kGTMSimpleThreadRunning];
-
-  while (![self isCancelled] &&
-         [runLock_ tryLockWhenCondition:kGTMSimpleThreadRunning]) {
-    [runLock_ unlock];
-    // We can't control nesting of runloops, so we spin with a short timeout. If
-    // another thread cancels us the CFRunloopStop() we might get it right away,
-    // if there is no nesting, otherwise our timeout will still get us to exit
-    // in reasonable time.
-    [loop runMode:NSDefaultRunLoopMode
-         beforeDate:[NSDate dateWithTimeIntervalSinceNow:1.0]];
-    [localPool drain];
-    localPool = [[NSAutoreleasePool alloc] init];
-  }
-
-  // Exit
-  [runLock_ lock];
-  [localPool drain];
-  if (runLoop_) CFRelease(runLoop_);
-  runLoop_ = NULL;
-  [runLock_ unlockWithCondition:kGTMSimpleThreadFinished];
-}
-
-- (void)start {
-  // Before we start the thread we need to make sure its not already running
-  // and that the lock is past kGTMSimpleThreadInitialized so an immediate
-  // stop is safe.
-  [runLock_ lock];
-  if ([runLock_ condition] != kGTMSimpleThreadInitialized) {
-    [runLock_ unlock];
-    return;
+  NSRunLoop *nsRunLoop = [NSRunLoop currentRunLoop];
+  // According to the NSRunLoop docs, a port must be added to the
+  // runloop to keep the loop alive, otherwise when you call
+  // runMode:beforeDate: it will immediately return with NO. We never
+  // send anything over this port, it's only here to keep the run loop
+  // looping.
+  [nsRunLoop addPort:[NSPort port] forMode:NSDefaultRunLoopMode];
+  while (true) {
+    if (self.isCancelled) {
+      break;
+    }
+    BOOL ranLoop = [nsRunLoop runMode:NSDefaultRunLoopMode
+                           beforeDate:[NSDate distantFuture]];
+    if (!ranLoop) {
+      break;
+    }
   }
-  [runLock_ unlockWithCondition:kGTMSimpleThreadStarting];
-  [super start];
 }
 
 - (void)cancel {
-  // NSThread appears to not propagate [... isCancelled] to our thread in
-  // this subclass, so we'll let super know and then use our condition lock.
   [super cancel];
-  [runLock_ lock];
-  switch ([runLock_ condition]) {
-    case kGTMSimpleThreadInitialized:
-    case kGTMSimpleThreadStarting:
-      // Cancelled before we started? Transition straight to finished.
-      [runLock_ unlockWithCondition:kGTMSimpleThreadFinished];
-      return;
-    case kGTMSimpleThreadRunning:
-      // If the thread has exited without changing lock state we detect that
-      // here. Note this is a direct call to [super isExecuting] to prevent
-      // deadlock on |runLock_| in [self isExecuting].
-      if (![super isExecuting]) {
-        // Thread died in some unanticipated way, clean up on its behalf.
-        if (runLoop_) {
-          CFRelease(runLoop_);
-          runLoop_ = NULL;
-        }
-        [runLock_ unlockWithCondition:kGTMSimpleThreadFinished];
-        return;
-      } else {
-        // We need to cancel the running loop. We'd like to stop the runloop
-        // right now if we can (but see the caveat above about nested runloops).
-        if (runLoop_) CFRunLoopStop(runLoop_);
-        [runLock_ unlockWithCondition:kGTMSimpleThreadCancel];
-        return;
-      }
-    case kGTMSimpleThreadCancel:
-    case kGTMSimpleThreadFinished:
-      // Already cancelled or finished. There's an outside chance the thread
-      // will have died now (imagine a [... dealloc] that calls pthread_exit())
-      // but we'll ignore those cases.
-      [runLock_ unlock];
-      return;
+  if (![[NSThread currentThread] isEqual:self]) {
+    // This call just forces the runloop in main to spin allowing main to see
+    // that the isCancelled flag has been set. Note that this is only really
+    // needed if there are no blocks/selectors in the queue for the thread. If
+    // there are other items to be processed in the queue, the next one will be
+    // executed and then the "cancel" will be seen in main, and it will exit
+    // (and the other blocks will be dropped).
+    [self performSelector:@selector(class)
+                 onThread:self
+               withObject:nil
+            waitUntilDone:NO];
   }
 }
 
 - (void)stop {
-  // Cancel does the heavy lifting...
-  [self cancel];
-
-  // If we're the current thread then the stop was called from within our
-  // own runloop and we need to return control now. [... main] will handle
-  // the shutdown on its own.
-  if ([[NSThread currentThread] isEqual:self]) return;
-
-  // From all other threads block till we're finished. Note that [... cancel]
-  // handles ensuring we will either already be in this state or transition
-  // there after thread exit.
-  [runLock_ lockWhenCondition:kGTMSimpleThreadFinished];
-  [runLock_ unlock];
-
-  // We could still be waiting for thread teardown at this point (lock is in
-  // the right state, but thread is not quite torn down), so spin till we say
-  // execution is complete (our implementation checks superclass).
-  while ([self isExecuting]) {
-    usleep(10);
-  }
-}
-
-- (void)setName:(NSString *)name {
-  if ([self isExecuting]) {
-    [self performSelector:@selector(setThreadDebuggerName:)
+  if ([[NSThread currentThread] isEqual:self]) {
+    [super cancel];
+  } else {
+    // This call forces the runloop in main to spin allowing main to see that
+    // the isCancelled flag has been set. Note that we explicitly want to send
+    // it to the thread to process so it is added to the end of the queue of
+    // blocks to be processed. 'stop' guarantees that all items in the queue
+    // will be processed before it ends.
+    [self performSelector:@selector(cancel)
                  onThread:self
-               withObject:name
+               withObject:nil
             waitUntilDone:YES];
+    while (![self isFinished] || [self isExecuting]) {
+      // Spin until the thread is really done.
+      usleep(10);
+    }
   }
-  [super setName:name];
-}
-
-- (BOOL)isCancelled {
-  if ([super isCancelled]) return YES;
-  BOOL cancelled = NO;
-  [runLock_ lock];
-  if ([runLock_ condition] == kGTMSimpleThreadCancel) {
-    cancelled = YES;
-  }
-  [runLock_ unlock];
-  return cancelled;
-}
-
-- (BOOL)isExecuting {
-  if ([super isExecuting]) return YES;
-  [runLock_ lock];
-  switch ([runLock_ condition]) {
-    case kGTMSimpleThreadStarting:
-      // While starting we may not be executing yet, but we'll pretend we are.
-      [runLock_ unlock];
-      return YES;
-    case kGTMSimpleThreadCancel:
-    case kGTMSimpleThreadRunning:
-      // Both of these imply we're running, but [super isExecuting] failed,
-      // so the thread died for other reasons. Clean up.
-      if (runLoop_) {
-        CFRelease(runLoop_);
-        runLoop_ = NULL;
-      }
-      [runLock_ unlockWithCondition:kGTMSimpleThreadFinished];
-      break;
-    default:
-      [runLock_ unlock];
-      break;
-  }
-  return NO;
-}
-
-- (BOOL)isFinished {
-  if ([super isFinished]) return YES;
-  if ([self isExecuting]) return NO;  // Will clean up dead thread.
-  BOOL finished = NO;
-  [runLock_ lock];
-  if ([runLock_ condition] == kGTMSimpleThreadFinished) {
-    finished = YES;
-  }
-  [runLock_ unlock];
-  return finished;
 }
 
 @end
diff --git a/Foundation/GTMNSThread+BlocksTest.m b/Foundation/GTMNSThread+BlocksTest.m
index 4b685fd..fc2923b 100644
--- a/Foundation/GTMNSThread+BlocksTest.m
+++ b/Foundation/GTMNSThread+BlocksTest.m
@@ -17,10 +17,13 @@
 //
 
 #import <pthread.h>
+
 #import "GTMSenTestCase.h"
 #import "GTMNSThread+Blocks.h"
 
-#import "GTMFoundationUnitTestingUtilities.h"
+static const NSTimeInterval kTestTimeout = 5;
+static const int kThreadMethodCounter = 5;
+static const int kThreadMethoduSleep = 10000;
 
 @interface GTMNSThread_BlocksTest : GTMTestCase {
  @private
@@ -36,60 +39,48 @@
 }
 
 - (void)tearDown {
-  [workerThread_ stop];
+  [workerThread_ cancel];
   [workerThread_ release];
 }
 
 - (void)testPerformBlockOnCurrentThread {
   NSThread *currentThread = [NSThread currentThread];
-
-  GTMUnitTestingBooleanRunLoopContext *context =
-      [GTMUnitTestingBooleanRunLoopContext context];
   __block NSThread *runThread = nil;
 
   // Straight block runs right away (no runloop spin)
-  runThread = nil;
-  [context setShouldStop:NO];
   [currentThread gtm_performBlock:^{
     runThread = [NSThread currentThread];
-    [context setShouldStop:YES];
   }];
   XCTAssertEqualObjects(runThread, currentThread);
-  XCTAssertTrue([context shouldStop]);
 
   // Block with waiting runs immediately as well.
   runThread = nil;
-  [context setShouldStop:NO];
   [currentThread gtm_performWaitingUntilDone:YES block:^{
     runThread = [NSThread currentThread];
-    [context setShouldStop:YES];
   }];
   XCTAssertEqualObjects(runThread, currentThread);
-  XCTAssertTrue([context shouldStop]);
 
   // Block without waiting requires a runloop spin.
   runThread = nil;
-  [context setShouldStop:NO];
+  XCTestExpectation *expectation =
+      [self expectationWithDescription:@"BlockRan"];
   [currentThread gtm_performWaitingUntilDone:NO block:^{
     runThread = [NSThread currentThread];
-    [context setShouldStop:YES];
+    [expectation fulfill];
   }];
-  XCTAssertTrue([[NSRunLoop currentRunLoop]
-                 gtm_runUpToSixtySecondsWithContext:context]);
+  [self waitForExpectationsWithTimeout:kTestTimeout handler:NULL];
   XCTAssertEqualObjects(runThread, currentThread);
-  XCTAssertTrue([context shouldStop]);
 }
 
 - (void)testPerformBlockInBackground {
-  GTMUnitTestingBooleanRunLoopContext *context =
-      [GTMUnitTestingBooleanRunLoopContext context];
+  XCTestExpectation *expectation =
+      [self expectationWithDescription:@"BlockRan"];
   __block NSThread *runThread = nil;
   [NSThread gtm_performBlockInBackground:^{
     runThread = [NSThread currentThread];
-    [context setShouldStop:YES];
+    [expectation fulfill];
   }];
-  XCTAssertTrue([[NSRunLoop currentRunLoop]
-                 gtm_runUpToSixtySecondsWithContext:context]);
+  [self waitForExpectationsWithTimeout:kTestTimeout handler:NULL];
   XCTAssertNotNil(runThread);
   XCTAssertNotEqualObjects(runThread, [NSThread currentThread]);
 }
@@ -100,152 +91,226 @@
   XCTAssertFalse([worker isExecuting]);
   XCTAssertFalse([worker isFinished]);
 
-  // Unstarted worker can be stopped without error.
-  [worker stop];
-  XCTAssertFalse([worker isExecuting]);
-  XCTAssertTrue([worker isFinished]);
 
-  // And can be stopped again
-  [worker stop];
+  // Unstarted worker can be cancelled without error.
+  [worker cancel];
   XCTAssertFalse([worker isExecuting]);
-  XCTAssertTrue([worker isFinished]);
+  XCTAssertFalse([worker isFinished]);
 
-  // A thread we start can be stopped with correct state.
-  worker = [[GTMSimpleWorkerThread alloc] init];
+  // And can be cancelled again
+  [worker cancel];
   XCTAssertFalse([worker isExecuting]);
   XCTAssertFalse([worker isFinished]);
-  [worker start];
-  XCTAssertTrue([worker isExecuting]);
-  XCTAssertFalse([worker isFinished]);
-  [worker stop];
-  XCTAssertFalse([worker isExecuting]);
-  XCTAssertTrue([worker isFinished]);
+  [worker release];
 
-  // A cancel is also honored
+  // A thread we start can be cancelled with correct state.
   worker = [[GTMSimpleWorkerThread alloc] init];
   XCTAssertFalse([worker isExecuting]);
   XCTAssertFalse([worker isFinished]);
+  XCTestExpectation *blockPerformed =
+      [self expectationWithDescription:@"BlockIsRunning"];
   [worker start];
+  [workerThread_ gtm_performWaitingUntilDone:YES block:^{
+    [blockPerformed fulfill];
+  }];
+  [self waitForExpectationsWithTimeout:kTestTimeout handler:NULL];
   XCTAssertTrue([worker isExecuting]);
+  XCTAssertFalse([worker isCancelled]);
   XCTAssertFalse([worker isFinished]);
+  NSPredicate *predicate =
+      [NSPredicate predicateWithBlock:^BOOL(id workerThread,
+                                            NSDictionary<NSString *,id> *opts) {
+    return (BOOL)(![workerThread isExecuting]);
+  }];
+  [self expectationForPredicate:predicate
+            evaluatedWithObject:worker
+                        handler:NULL];
+
   [worker cancel];
-  // And after some time we're done. We're generous here, this needs to
-  // exceed the worker thread's runloop timeout.
-  sleep(5);
+  [self waitForExpectationsWithTimeout:kTestTimeout handler:NULL];
   XCTAssertFalse([worker isExecuting]);
+  XCTAssertTrue([worker isCancelled]);
   XCTAssertTrue([worker isFinished]);
-}
-
-- (void)testWorkerThreadStopTiming {
-  // Throw a sleep and make sure that we stop as soon as we can.
-  NSDate *start = [NSDate date];
-  NSConditionLock *threadLock = [[[NSConditionLock alloc] initWithCondition:0]
-                                    autorelease];
-  [workerThread_ gtm_performBlock:^{
-    [threadLock lock];
-    [threadLock unlockWithCondition:1];
-    [NSThread sleepForTimeInterval:.25];
-  }];
-  [threadLock lockWhenCondition:1];
-  [threadLock unlock];
-  [workerThread_ stop];
-  XCTAssertFalse([workerThread_ isExecuting]);
-  XCTAssertTrue([workerThread_ isFinished]);
-  XCTAssertEqualWithAccuracy(-[start timeIntervalSinceNow], 0.25, 0.25);
+  [worker release];
 }
 
 - (void)testPerformBlockOnWorkerThread {
-  GTMUnitTestingBooleanRunLoopContext *context =
-      [GTMUnitTestingBooleanRunLoopContext context];
   __block NSThread *runThread = nil;
 
   // Runs on the other thread
-  runThread = nil;
-  [context setShouldStop:NO];
+  XCTestExpectation *expectation =
+      [self expectationWithDescription:@"BlockRan"];
   [workerThread_ gtm_performBlock:^{
     runThread = [NSThread currentThread];
-    [context setShouldStop:YES];
+    [expectation fulfill];
   }];
-  XCTAssertTrue([[NSRunLoop currentRunLoop]
-                 gtm_runUpToSixtySecondsWithContext:context]);
+  [self waitForExpectationsWithTimeout:kTestTimeout handler:NULL];
   XCTAssertNotNil(runThread);
   XCTAssertEqualObjects(runThread, workerThread_);
 
   // Other thread no wait.
   runThread = nil;
-  [context setShouldStop:NO];
+  expectation = [self expectationWithDescription:@"BlockRan2"];
   [workerThread_ gtm_performWaitingUntilDone:NO block:^{
     runThread = [NSThread currentThread];
-    [context setShouldStop:YES];
+    [expectation fulfill];
   }];
-  XCTAssertTrue([[NSRunLoop currentRunLoop]
-                 gtm_runUpToSixtySecondsWithContext:context]);
+  [self waitForExpectationsWithTimeout:kTestTimeout handler:NULL];
   XCTAssertNotNil(runThread);
   XCTAssertEqualObjects(runThread, workerThread_);
 
   // Waiting requires no runloop spin
   runThread = nil;
-  [context setShouldStop:NO];
   [workerThread_ gtm_performWaitingUntilDone:YES block:^{
     runThread = [NSThread currentThread];
-    [context setShouldStop:YES];
   }];
-  XCTAssertTrue([context shouldStop]);
   XCTAssertNotNil(runThread);
   XCTAssertEqualObjects(runThread, workerThread_);
 }
 
-- (void)testExitingBlockIsExecuting {
-  NSConditionLock *threadLock = [[[NSConditionLock alloc] initWithCondition:0]
-                                    autorelease];
+- (void)testExitingBlock {
   [workerThread_ gtm_performWaitingUntilDone:NO block:^{
-    [threadLock lock];
-    [threadLock unlockWithCondition:1];
-    pthread_exit(NULL);
+     pthread_exit(NULL);
+  }];
+  NSPredicate *predicate =
+      [NSPredicate predicateWithBlock:^BOOL(id workerThread,
+                                            NSDictionary<NSString *,id> *opts) {
+    return (BOOL)(![workerThread isExecuting]);
   }];
-  [threadLock lockWhenCondition:1];
-  [threadLock unlock];
-  // Give the pthread_exit() a bit of time
-  [NSThread sleepForTimeInterval:.25];
-  // Did we notice the thread died? Does [... isExecuting] clean up?
-  XCTAssertFalse([workerThread_ isExecuting]);
+  [self expectationForPredicate:predicate
+            evaluatedWithObject:workerThread_
+                        handler:NULL];
+  [self waitForExpectationsWithTimeout:kTestTimeout handler:NULL];
   XCTAssertTrue([workerThread_ isFinished]);
 }
 
-- (void)testExitingBlockCancel {
-  NSConditionLock *threadLock = [[[NSConditionLock alloc] initWithCondition:0]
-                                    autorelease];
+- (void)testCancelFromThread {
   [workerThread_ gtm_performWaitingUntilDone:NO block:^{
-    [threadLock lock];
-    [threadLock unlockWithCondition:1];
-    pthread_exit(NULL);
+    [workerThread_ cancel];
   }];
-  [threadLock lockWhenCondition:1];
-  [threadLock unlock];
-  // Give the pthread_exit() a bit of time
-  [NSThread sleepForTimeInterval:.25];
-  // Cancel/stop the thread
-  [workerThread_ stop];
-  // Did we notice the thread died? Did we clean up?
-  XCTAssertFalse([workerThread_ isExecuting]);
+  NSPredicate *predicate =
+      [NSPredicate predicateWithBlock:^BOOL(id workerThread,
+                                            NSDictionary<NSString *,id> *opts) {
+    return (BOOL)(![workerThread isExecuting]);
+  }];
+  [self expectationForPredicate:predicate
+            evaluatedWithObject:workerThread_
+                        handler:NULL];
+  [self waitForExpectationsWithTimeout:kTestTimeout handler:NULL];
   XCTAssertTrue([workerThread_ isFinished]);
 }
 
+- (void)testNestedCancelFromThread {
+  [workerThread_ gtm_performWaitingUntilDone:NO block:^{
+    [workerThread_ gtm_performWaitingUntilDone:NO block:^{
+      [workerThread_ gtm_performWaitingUntilDone:NO block:^{
+        [workerThread_ gtm_performWaitingUntilDone:NO block:^{
+          [workerThread_ cancel];
+        }];
+      }];
+    }];
+  }];
+  NSPredicate *predicate =
+      [NSPredicate predicateWithBlock:^BOOL(id workerThread,
+                                            NSDictionary<NSString *,id> *opts) {
+    return (BOOL)(![workerThread isExecuting]);
+  }];
+  [self expectationForPredicate:predicate
+            evaluatedWithObject:workerThread_
+                        handler:NULL];
+  [self waitForExpectationsWithTimeout:kTestTimeout handler:NULL];
+  XCTAssertTrue([workerThread_ isFinished]);
+}
+
+- (void)testCancelFromOtherThread {
+  // Show that cancel actually cancels before all blocks are executed.
+  __block int counter = 0;
+  for (int i = 0; i < kThreadMethodCounter; i++) {
+    [workerThread_ gtm_performWaitingUntilDone:NO block:^{
+      sleep(1);
+      ++counter;
+    }];
+  }
+  [workerThread_ cancel];
+  NSPredicate *predicate =
+      [NSPredicate predicateWithBlock:^BOOL(id workerThread,
+                                            NSDictionary<NSString *,id> *opts) {
+    return (BOOL)(![workerThread isExecuting]);
+  }];
+  [self expectationForPredicate:predicate
+            evaluatedWithObject:workerThread_
+                        handler:NULL];
+  [self waitForExpectationsWithTimeout:kTestTimeout handler:NULL];
+  XCTAssertTrue([workerThread_ isFinished]);
+  XCTAssertNotEqual(counter, kThreadMethodCounter);
+}
+
 - (void)testStopFromThread {
-  NSConditionLock *threadLock = [[[NSConditionLock alloc] initWithCondition:0]
-                                    autorelease];
+  // Show that stop forces all blocks to be executed.
+  __block int counter = 0;
+  for (int i = 0; i < kThreadMethodCounter; i++) {
+    [workerThread_ gtm_performWaitingUntilDone:NO block:^{
+      usleep(kThreadMethoduSleep);
+      ++counter;
+    }];
+  }
+  [workerThread_ gtm_performWaitingUntilDone:NO block:^{
+    [workerThread_ stop];
+  }];
+  NSPredicate *predicate =
+      [NSPredicate predicateWithBlock:^BOOL(id workerThread,
+                                            NSDictionary<NSString *,id> *opts) {
+      return (BOOL)(![workerThread isExecuting]);
+  }];
+  [self expectationForPredicate:predicate
+            evaluatedWithObject:workerThread_
+                        handler:NULL];
+  [self waitForExpectationsWithTimeout:kTestTimeout handler:NULL];
+  XCTAssertTrue([workerThread_ isFinished]);
+  XCTAssertEqual(counter, kThreadMethodCounter);
+}
+
+- (void)testNestedStopFromThread {
+  __block int counter = 0;
+  for (int i = 0; i < kThreadMethodCounter; i++) {
+    [workerThread_ gtm_performWaitingUntilDone:NO block:^{
+      usleep(kThreadMethoduSleep);
+      ++counter;
+    }];
+  }
   [workerThread_ gtm_performWaitingUntilDone:NO block:^{
-    [threadLock lock];
-    [workerThread_ stop];  // Shold not block.
-    [threadLock unlockWithCondition:1];
+    [workerThread_ gtm_performWaitingUntilDone:NO block:^{
+      [workerThread_ gtm_performWaitingUntilDone:NO block:^{
+        [workerThread_ gtm_performWaitingUntilDone:NO block:^{
+          [workerThread_ stop];
+        }];
+      }];
+    }];
+  }];
+  NSPredicate *predicate =
+      [NSPredicate predicateWithBlock:^BOOL(id workerThread,
+                                            NSDictionary<NSString *,id> *opts) {
+    return (BOOL)(![workerThread isExecuting]);
   }];
-  // Block should complete before the stop occurs.
-  [threadLock lockWhenCondition:1];
-  [threadLock unlock];
-  // Still need to give the thread a moment to not be executing
-  sleep(1);
-  XCTAssertFalse([workerThread_ isExecuting]);
+  [self expectationForPredicate:predicate
+            evaluatedWithObject:workerThread_
+                        handler:NULL];
+  [self waitForExpectationsWithTimeout:kTestTimeout handler:NULL];
+  XCTAssertTrue([workerThread_ isFinished]);
+  XCTAssertEqual(counter, kThreadMethodCounter);
+}
+
+- (void)testStopFromOtherThread {
+  __block int counter = 0;
+  for (int i = 0; i < kThreadMethodCounter; i++) {
+    [workerThread_ gtm_performWaitingUntilDone:NO block:^{
+      usleep(kThreadMethoduSleep);
+      ++counter;
+    }];
+  }
+  [workerThread_ stop];
   XCTAssertTrue([workerThread_ isFinished]);
+  XCTAssertEqual(counter, kThreadMethodCounter);
 }
 
 - (void)testPThreadName {