Fixed up GTMSignalHandler and deprecated it. (#192)

This gets GTMSignalHandler working with libdispatch, and also marks it
as deprecated because it is probably easier just to write your own
version to call a block instead of the method invocation here.
This also makes the tests run again which hasn't been happening for
a long time AFAICT.

4 files changed, 110 insertions, 189 deletions

diff --git a/Foundation/GTMSignalHandler.h b/Foundation/GTMSignalHandler.h
index 5f4b028..eec0e24 100644
--- a/Foundation/GTMSignalHandler.h
+++ b/Foundation/GTMSignalHandler.h
@@ -23,7 +23,7 @@
 //
 // This is a very simple, easy-to-use class for registering handlers that get
 // called when a specific signal is delivered.  Also handy for ignoring
-// inconvenient signals.  Ignoring SIGKILL is not support for what should be
+// inconvenient signals.  Ignoring SIGKILL is not supported for what should be
 // obvious reasons.  You can pass nil for target & action to ignore the signal.
 //
 // Example of how to catch SIGABRT and SIGTERM while ignring SIGWINCH:
@@ -53,16 +53,15 @@
 //
 // Multiple handlers for the same signal is NOT supported.
 //
-// kqueue() is used to handle the signals, and the default runloop for the first
-// signal handler is used for signal delivery, so keep that in mind when you're
-// using this class.  This class explicitly does not handle arbitrary runloops
-// and threading.
-//
+// libdispatch is used to handle the signals. NB that sending a signal
+// from dispatch_get_main_queue will not get caught in a CFRunLoop based run
+// loop.
+// https://openradar.appspot.com/radar?id=5030997057863680
+
+NS_DEPRECATED(10_0, 10_6, 1_0, 4_0, "Use a DISPATCH_SOURCE_TYPE_SIGNAL dispatch source.")
 @interface GTMSignalHandler : NSObject {
  @private
-  int signo_;
-  GTM_WEAK id target_;
-  SEL action_;
+  dispatch_source_t signalSource_;
 }
 
 // Returns a retained signal handler object that will invoke |handler| on the
diff --git a/Foundation/GTMSignalHandler.m b/Foundation/GTMSignalHandler.m
index 05ee826..4d4f3e2 100644
--- a/Foundation/GTMSignalHandler.m
+++ b/Foundation/GTMSignalHandler.m
@@ -18,9 +18,8 @@
 
 #import "GTMSignalHandler.h"
 #import "GTMDefines.h"
-#import "GTMTypeCasting.h"
 
-#import <sys/event.h>  // for kqueue() and kevent
+#import <dispatch/dispatch.h>
 #import "GTMDebugSelectorValidation.h"
 
 // Simplifying assumption: No more than one handler for a particular signal is
@@ -31,22 +30,6 @@
 // handlers interested in a particular signal, but not really worth it for apps
 // that register the handlers at startup and don't change them.
 
-
-// File descriptor for the kqueue that will hold all of our signal events.
-static int gSignalKQueueFileDescriptor = 0;
-
-// A wrapper around the kqueue file descriptor so we can put it into a
-// runloop.
-static CFSocketRef gRunLoopSocket = NULL;
-
-
-@interface GTMSignalHandler (PrivateMethods)
-- (void)notify;
-- (void)addFileDescriptorMonitor:(int)fd;
-- (void)registerWithKQueue;
-@end
-
-
 @implementation GTMSignalHandler
 
 -(id)init {
@@ -67,20 +50,34 @@ static CFSocketRef gRunLoopSocket = NULL;
       return nil;
     }
 
-    signo_ = signo;
-    target_ = target;  // Don't retain since target will most likely retain us.
-    action_ = action;
-    GTMAssertSelectorNilOrImplementedWithArguments(target_,
-                                                   action_,
+    GTMAssertSelectorNilOrImplementedWithArguments(target,
+                                                   action,
                                                    @encode(int),
                                                    NULL);
 
-    // We're handling this signal via kqueue, so turn off the usual signal
+    // We're handling this signal via libdispatch, so turn off the usual signal
     // handling.
-    signal(signo_, SIG_IGN);
+    if (signal(signo, SIG_IGN) == SIG_ERR) {
+      _GTMDevLog(@"could not ignore signal %d.  Errno %d", signo, errno);  // COV_NF_LINE
+    }
 
     if (action != NULL) {
-      [self registerWithKQueue];
+      signalSource_ = dispatch_source_create(DISPATCH_SOURCE_TYPE_SIGNAL,
+                                             signo,
+                                             0,
+                                             dispatch_get_main_queue());
+      dispatch_source_set_event_handler(signalSource_, ^(void) {
+        NSMethodSignature *methodSig
+            = [target methodSignatureForSelector:action];
+        _GTMDevAssert(methodSig != nil, @"failed to get the signature?");
+        NSInvocation *invocation
+            = [NSInvocation invocationWithMethodSignature:methodSig];
+        [invocation setTarget:target];
+        [invocation setSelector:action];
+        [invocation setArgument:(void*)&signo atIndex:2];
+        [invocation invoke];
+      });
+      dispatch_resume(signalSource_);
     }
   }
   return self;
@@ -91,112 +88,13 @@ static CFSocketRef gRunLoopSocket = NULL;
   [super dealloc];
 }
 
-// Cribbed from Advanced Mac OS X Programming.
-static void SocketCallBack(CFSocketRef socketref, CFSocketCallBackType type,
-                           CFDataRef address, const void *data, void *info) {
-  // We're using CFRunLoop calls here. Even when used on the main thread, they
-  // don't trigger the draining of the main application's autorelease pool that
-  // NSRunLoop provides. If we're used in a UI-less app, this means that
-  // autoreleased objects would never go away, so we provide our own pool here.
-  NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
-
-  struct kevent event;
-
-  if (kevent(gSignalKQueueFileDescriptor, NULL, 0, &event, 1, NULL) == -1) {
-    _GTMDevLog(@"could not pick up kqueue event.  Errno %d", errno);  // COV_NF_LINE
-  } else {
-    GTMSignalHandler *handler = GTM_STATIC_CAST(GTMSignalHandler, event.udata);
-    [handler notify];
-  }
-
-  [pool drain];
-}
-
-// Cribbed from Advanced Mac OS X Programming
-- (void)addFileDescriptorMonitor:(int)fd {
-  CFSocketContext context = { 0, NULL, NULL, NULL, NULL };
-
-  gRunLoopSocket = CFSocketCreateWithNative(kCFAllocatorDefault,
-                                            fd,
-                                            kCFSocketReadCallBack,
-                                            SocketCallBack,
-                                            &context);
-  if (gRunLoopSocket == NULL) {
-    _GTMDevLog(@"could not CFSocketCreateWithNative");  // COV_NF_LINE
-    goto bailout;   // COV_NF_LINE
-  }
-
-  CFRunLoopSourceRef rls;
-  rls = CFSocketCreateRunLoopSource(NULL, gRunLoopSocket, 0);
-  if (rls == NULL) {
-    _GTMDevLog(@"could not create a run loop source");  // COV_NF_LINE
-    goto bailout;  // COV_NF_LINE
-  }
-
-  CFRunLoopAddSource(CFRunLoopGetCurrent(), rls,
-                     kCFRunLoopDefaultMode);
-  CFRelease(rls);
-
- bailout:
-  return;
-
-}
-
-- (void)registerWithKQueue {
-
-  // Make sure we have our kqueue.
-  if (gSignalKQueueFileDescriptor == 0) {
-    gSignalKQueueFileDescriptor = kqueue();
-
-    if (gSignalKQueueFileDescriptor == -1) {
-      _GTMDevLog(@"could not make signal kqueue.  Errno %d", errno);  // COV_NF_LINE
-      return;  // COV_NF_LINE
-    }
-
-    // Add the kqueue file descriptor to the runloop.
-    [self addFileDescriptorMonitor:gSignalKQueueFileDescriptor];
-  }
-
-  // Add a new event for the signal.
-  struct kevent filter;
-  EV_SET(&filter, signo_, EVFILT_SIGNAL, EV_ADD | EV_ENABLE | EV_CLEAR,
-         0, 0, self);
-
-  const struct timespec noWait = { 0, 0 };
-  if (kevent(gSignalKQueueFileDescriptor, &filter, 1, NULL, 0, &noWait) != 0) {
-    _GTMDevLog(@"could not add event for signal %d.  Errno %d", signo_, errno);  // COV_NF_LINE
-  }
-
-}
 
 - (void)invalidate {
-  // Short-circuit cases where we didn't actually register a kqueue event.
-  if (signo_ == 0) return;
-  if (action_ == nil) return;
-
-  struct kevent filter;
-  EV_SET(&filter, signo_, EVFILT_SIGNAL, EV_DELETE, 0, 0, self);
-
-  const struct timespec noWait = { 0, 0 };
-  if (kevent(gSignalKQueueFileDescriptor, &filter, 1, NULL, 0, &noWait) != 0) {
-    _GTMDevLog(@"could not remove event for signal %d.  Errno %d", signo_, errno);  // COV_NF_LINE
+  if (signalSource_) {
+    dispatch_source_cancel(signalSource_);
+    dispatch_release(signalSource_);
+    signalSource_ = NULL;
   }
-
-  // Set action_ to nil so that if invalidate is called on us twice,
-  // nothing happens.
-  action_ = nil;
-}
-
-- (void)notify {
-  // Now, fire the selector
-  NSMethodSignature *methodSig = [target_ methodSignatureForSelector:action_];
-  _GTMDevAssert(methodSig != nil, @"failed to get the signature?");
-  NSInvocation *invocation
-    = [NSInvocation invocationWithMethodSignature:methodSig];
-  [invocation setTarget:target_];
-  [invocation setSelector:action_];
-  [invocation setArgument:&signo_ atIndex:2];
-  [invocation invoke];
 }
 
 @end
diff --git a/Foundation/GTMSignalHandlerTest.m b/Foundation/GTMSignalHandlerTest.m
index bd70ecf..d2b52ca 100644
--- a/Foundation/GTMSignalHandlerTest.m
+++ b/Foundation/GTMSignalHandlerTest.m
@@ -18,47 +18,41 @@
 
 #import "GTMSenTestCase.h"
 #import "GTMSignalHandler.h"
-#import "GTMFoundationUnitTestingUtilities.h"
+
+#pragma clang diagnostic push
+// Ignore all of the deprecation warnings for GTMRegex
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
 
 @interface GTMSignalHandlerTest : GTMTestCase
 @end
 
-@interface SignalCounter : NSObject<GTMUnitTestingRunLoopContext> {
- @public
+@interface SignalCounter : NSObject {
   int signalCount_;
   int lastSeenSignal_;
-  BOOL shouldStop_;
 }
 - (int)count;
 - (int)lastSeen;
 - (void)countSignal:(int)signo;
 + (id)signalCounter;
-- (void)resetShouldStop;
 @end // SignalCounter
 
 @implementation SignalCounter
 + (id)signalCounter {
   return [[[self alloc] init] autorelease];
 }
+
 - (int)count {
   return signalCount_;
 }
+
 - (int)lastSeen {
   return lastSeenSignal_;
 }
+
 // Count the number of times this signal handler has fired.
 - (void)countSignal:(int)signo {
   signalCount_++;
   lastSeenSignal_ = signo;
-  shouldStop_ = YES;
-}
-
-- (BOOL)shouldStop {
-  return shouldStop_;
-}
-
-- (void)resetShouldStop {
-  shouldStop_ = NO;
 }
 
 @end
@@ -85,7 +79,18 @@
 }
 
 - (void)testSingleHandler {
+  // SIGIO and SIGWINCH were chosen for this test because LLDB does not trap
+  // them which allows you to run this test under the debugger.
+  // If you need to use other signals and the debugger is getting annoying
+  // https://stackoverflow.com/questions/11984051/how-to-tell-lldb-debugger-not-to-handle-sigbus
   SignalCounter *counter = [SignalCounter signalCounter];
+
+  // Raising our signals off of a background queue becuase raising them
+  // off of dispatch_main_queue does not work with a CFRunLoop.
+  // https://openradar.appspot.com/radar?id=5030997057863680
+  dispatch_queue_t raiseQueue =
+      dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
+  const int deltaT = 5000000;
   XCTAssertNotNil(counter);
 
   GTMSignalHandler *handler = [[[GTMSignalHandler alloc]
@@ -94,64 +99,81 @@
                                          action:@selector(countSignal:)]
                                autorelease];
   XCTAssertNotNil(handler);
-  raise(SIGWINCH);
-
-  NSRunLoop *rl = [NSRunLoop currentRunLoop];
-  [rl gtm_runUpToSixtySecondsWithContext:counter];
-
-  XCTAssertEqual([counter count], 1);
-  XCTAssertEqual([counter lastSeen], SIGWINCH);
-  [counter resetShouldStop];
 
-  raise(SIGWINCH);
-  [rl gtm_runUpToSixtySecondsWithContext:counter];
-
-  XCTAssertEqual([counter count], 2);
-  XCTAssertEqual([counter lastSeen], SIGWINCH);
-  [counter resetShouldStop];
-
-  // create a second one to make sure we're seding data where we want
+  [self expectationForPredicate:
+       [NSPredicate predicateWithFormat:@"self.lastSeen == %d", SIGWINCH]
+            evaluatedWithObject:counter handler:NULL];
+  [self expectationForPredicate:
+       [NSPredicate predicateWithFormat:@"self.count == 1"]
+            evaluatedWithObject:counter handler:NULL];
+  dispatch_after(dispatch_time(DISPATCH_TIME_NOW, deltaT), raiseQueue, ^{
+    // Using dispatch_after to make sure our signal is sent AFTER the runloop
+    // is being spun in waitForExpectationsWithTimeout.
+    raise(SIGWINCH);
+  });
+  [self waitForExpectationsWithTimeout:5 handler:NULL];
+  [self expectationForPredicate:
+       [NSPredicate predicateWithFormat:@"self.lastSeen == %d", SIGWINCH]
+            evaluatedWithObject:counter handler:NULL];
+  [self expectationForPredicate:
+       [NSPredicate predicateWithFormat:@"self.count == 2"]
+            evaluatedWithObject:counter handler:NULL];
+
+  dispatch_after(dispatch_time(DISPATCH_TIME_NOW, deltaT), raiseQueue, ^{
+    raise(SIGWINCH);
+  });
+  [self waitForExpectationsWithTimeout:5 handler:NULL];
+
+  // create a second one to make sure we're sending data where we want
   SignalCounter *counter2 = [SignalCounter signalCounter];
   XCTAssertNotNil(counter2);
-  [[[GTMSignalHandler alloc] initWithSignal:SIGUSR1
-                                     target:counter2
-                                     action:@selector(countSignal:)] autorelease];
-
-  raise(SIGUSR1);
-  [rl gtm_runUpToSixtySecondsWithContext:counter2];
+  [[[GTMSignalHandler alloc] initWithSignal:SIGIO
+      target:counter2
+      action:@selector(countSignal:)] autorelease];
+  [self expectationForPredicate:
+       [NSPredicate predicateWithFormat:@"self.lastSeen == %d", SIGIO]
+            evaluatedWithObject:counter2 handler:NULL];
+  [self expectationForPredicate:
+       [NSPredicate predicateWithFormat:@"self.count == 1"]
+            evaluatedWithObject:counter2 handler:NULL];
+
+  dispatch_after(dispatch_time(DISPATCH_TIME_NOW, deltaT), raiseQueue, ^{
+    raise(SIGIO);
+  });
+  [self waitForExpectationsWithTimeout:5 handler:NULL];
 
   XCTAssertEqual([counter count], 2);
   XCTAssertEqual([counter lastSeen], SIGWINCH);
-  XCTAssertEqual([counter2 count], 1);
-  XCTAssertEqual([counter2 lastSeen], SIGUSR1);
 
   [handler invalidate];
 
   // The signal is still ignored (so we shouldn't die), but the
   // the handler method should not get called.
+  [self expectationForPredicate:
+       [NSPredicate predicateWithFormat:@"self.lastSeen == %d", SIGWINCH]
+            evaluatedWithObject:counter handler:NULL].inverted = YES;
+  [self expectationForPredicate:
+      [NSPredicate predicateWithFormat:@"self.count == 2"]
+            evaluatedWithObject:counter handler:NULL].inverted = YES;
   raise(SIGWINCH);
-  [rl runUntilDate:[NSDate dateWithTimeIntervalSinceNow:.2]];
-
-  XCTAssertEqual([counter count], 2);
-  XCTAssertEqual([counter lastSeen], SIGWINCH);
-  XCTAssertEqual([counter2 count], 1);
-  XCTAssertEqual([counter2 lastSeen], SIGUSR1);
-
+  [self waitForExpectationsWithTimeout:.2 handler:NULL];
 }
 
 - (void)testIgnore {
   SignalCounter *counter = [SignalCounter signalCounter];
   XCTAssertNotNil(counter);
 
-  [[[GTMSignalHandler alloc] initWithSignal:SIGUSR1
+  [[[GTMSignalHandler alloc] initWithSignal:SIGIO
                                      target:counter
                                      action:NULL] autorelease];
 
-  raise(SIGUSR1);
-  NSRunLoop *rl = [NSRunLoop currentRunLoop];
-  [rl runUntilDate:[NSDate dateWithTimeIntervalSinceNow:.2]];
-  XCTAssertEqual([counter count], 0);
-
+  [self expectationForPredicate:
+       [NSPredicate predicateWithFormat:@"self.count == 0"]
+            evaluatedWithObject:counter handler:NULL].inverted = YES;
+  raise(SIGIO);
+  [self waitForExpectationsWithTimeout:.2 handler:NULL];
 }
 
 @end
+
+#pragma clang diagnostic pop
diff --git a/GTM.xcodeproj/project.pbxproj b/GTM.xcodeproj/project.pbxproj
index 01ced21..92dedf1 100644
--- a/GTM.xcodeproj/project.pbxproj
+++ b/GTM.xcodeproj/project.pbxproj
@@ -25,6 +25,7 @@
 		8B07C60F1D99B01D0054728B /* XCTest.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 8B07C60D1D99B01D0054728B /* XCTest.framework */; };
 		8B07C6101D99B01D0054728B /* XCTest.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 8B07C60D1D99B01D0054728B /* XCTest.framework */; };
 		8B07C6111D99B01D0054728B /* XCTest.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 8B07C60D1D99B01D0054728B /* XCTest.framework */; };
+		8B0D4638219A4E5D00B6D4A7 /* GTMSignalHandlerTest.m in Sources */ = {isa = PBXBuildFile; fileRef = F41A6F810E02EC3600788A6C /* GTMSignalHandlerTest.m */; };
 		8B158A9B10A8C31100C93125 /* GTMNSAnimation+Duration.m in Sources */ = {isa = PBXBuildFile; fileRef = 8B158A6010A8BE4500C93125 /* GTMNSAnimation+Duration.m */; };
 		8B158ADE10A8C42000C93125 /* GTMNSAnimation+Duration.h in Headers */ = {isa = PBXBuildFile; fileRef = 8B158A5F10A8BE4500C93125 /* GTMNSAnimation+Duration.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		8B17FD15117638D500E7A908 /* GTMFoundationUnitTestingUtilities.m in Sources */ = {isa = PBXBuildFile; fileRef = 8B17FD13117638D500E7A908 /* GTMFoundationUnitTestingUtilities.m */; };
@@ -1146,6 +1147,7 @@
 				8BFE6E821282371200B5C894 /* GTMLogger+ASLTest.m in Sources */,
 				8BFE6E831282371200B5C894 /* GTMLoggerRingBufferWriterTest.m in Sources */,
 				8BFE6E841282371200B5C894 /* GTMLoggerTest.m in Sources */,
+				8B0D4638219A4E5D00B6D4A7 /* GTMSignalHandlerTest.m in Sources */,
 				8BFE6E891282371200B5C894 /* GTMNSData+zlibTest.m in Sources */,
 				8BFE6E8B1282371200B5C894 /* GTMNSDictionary+URLArgumentsTest.m in Sources */,
 				8BFE6E8D1282371200B5C894 /* GTMNSFileHandle+UniqueNameTest.m in Sources */,