GULLogger - issue count synchronous getters (#2610)

Author: maksymmalyhin

GoogleUtilities/Example/Tests/Logger/GULLoggerTest.m

@@ -32,15 +32,15 @@
 
 extern BOOL getGULLoggerDebugMode(void);
 
-extern NSUserDefaults *getGULLoggerUsetDefaults(void);
+extern CFStringRef getGULLoggerUsetDefaultsSuiteName(void);
+extern dispatch_queue_t getGULLoggerCounterQueue(void);
 
 static NSString *const kMessageCode = @"I-COR000001";
 
 @interface GULLoggerTest : XCTestCase
 
 @property(nonatomic) NSString *randomLogString;
-
-@property(nonatomic, strong) NSUserDefaults *defaults;
+@property(nonatomic) NSUserDefaults *loggerDefaults;
 
 @end
 
@@ -50,14 +50,18 @@ - (void)setUp {
   [super setUp];
   GULResetLogger();
 
-  // Stub NSUserDefaults for cleaner testing.
-  _defaults = [[NSUserDefaults alloc] initWithSuiteName:@"com.google.logger_test"];
+  self.loggerDefaults = [[NSUserDefaults alloc]
+      initWithSuiteName:CFBridgingRelease(getGULLoggerUsetDefaultsSuiteName())];
 }
 
 - (void)tearDown {
-  [super tearDown];
+  // Make sure all async operations have finished before starting a new test.
+  [self drainQueue:getGULClientQueue()];
+  [self drainQueue:getGULLoggerCounterQueue()];
 
-  _defaults = nil;
+  self.loggerDefaults = nil;
+
+  [super tearDown];
 }
 
 - (void)testMessageCodeFormat {
@@ -158,93 +162,58 @@ - (void)testGULLoggerLevelValues {
 - (void)testGetErrorWarningNumberBeforeLogDontCrash {
   GULResetLogger();
 
-  XCTestExpectation *getErrorCountExpectation =
-      [self expectationWithDescription:@"getErrorCountExpectation"];
-  XCTestExpectation *getWarningsCountExpectation =
-      [self expectationWithDescription:@"getWarningsCountExpectation"];
-
-  GULNumberOfErrorsLogged(^(NSInteger count) {
-    [getErrorCountExpectation fulfill];
-  });
-
-  GULNumberOfWarningsLogged(^(NSInteger count) {
-    [getWarningsCountExpectation fulfill];
-  });
-
-  [self waitForExpectations:@[ getErrorCountExpectation, getWarningsCountExpectation ]
-                    timeout:0.5
-               enforceOrder:YES];
+  XCTAssertNoThrow(GULNumberOfErrorsLogged());
+  XCTAssertNoThrow(GULNumberOfWarningsLogged());
 }
 
 - (void)testErrorNumberIncrement {
-  [getGULLoggerUsetDefaults() setInteger:10 forKey:kGULLoggerErrorCountKey];
+  [self.loggerDefaults setInteger:10 forKey:kGULLoggerErrorCountKey];
 
   GULLogError(@"my service", NO, kMessageCode, @"Message.");
 
-  XCTestExpectation *getErrorCountExpectation =
-      [self expectationWithDescription:@"getErrorCountExpectation"];
-
-  GULNumberOfErrorsLogged(^(NSInteger count) {
-    [getErrorCountExpectation fulfill];
-    XCTAssertEqual(count, 11);
-  });
-
-  [self waitForExpectationsWithTimeout:0.5 handler:NULL];
+  [self drainQueue:getGULLoggerCounterQueue()];
+  XCTAssertEqual(GULNumberOfErrorsLogged(), 11);
 }
 
 - (void)testWarningNumberIncrement {
-  [getGULLoggerUsetDefaults() setInteger:5 forKey:kGULLoggerWarningCountKey];
+  [self.loggerDefaults setInteger:5 forKey:kGULLoggerWarningCountKey];
 
   GULLogWarning(@"my service", NO, kMessageCode, @"Message.");
 
-  XCTestExpectation *getWarningsCountExpectation =
-      [self expectationWithDescription:@"getWarningsCountExpectation"];
-
-  GULNumberOfWarningsLogged(^(NSInteger count) {
-    [getWarningsCountExpectation fulfill];
-    XCTAssertEqual(count, 6);
-  });
-
-  [self waitForExpectationsWithTimeout:0.5 handler:NULL];
+  [self drainQueue:getGULLoggerCounterQueue()];
+  XCTAssertEqual(GULNumberOfWarningsLogged(), 6);
 }
 
 - (void)testResetIssuesCount {
-  [getGULLoggerUsetDefaults() setInteger:3 forKey:kGULLoggerErrorCountKey];
-  [getGULLoggerUsetDefaults() setInteger:4 forKey:kGULLoggerWarningCountKey];
+  [self.loggerDefaults setInteger:3 forKey:kGULLoggerErrorCountKey];
+  [self.loggerDefaults setInteger:4 forKey:kGULLoggerWarningCountKey];
 
   GULResetNumberOfIssuesLogged();
 
-  XCTestExpectation *getErrorCountExpectation =
-      [self expectationWithDescription:@"getErrorCountExpectation"];
-  XCTestExpectation *getWarningsCountExpectation =
-      [self expectationWithDescription:@"getWarningsCountExpectation"];
-
-  GULNumberOfErrorsLogged(^(NSInteger count) {
-    [getErrorCountExpectation fulfill];
-    XCTAssertEqual(count, 0);
-  });
-
-  GULNumberOfWarningsLogged(^(NSInteger count) {
-    [getWarningsCountExpectation fulfill];
-    XCTAssertEqual(count, 0);
-  });
+  XCTAssertEqual(GULNumberOfErrorsLogged(), 0);
+  XCTAssertEqual(GULNumberOfWarningsLogged(), 0);
+}
 
-  [self waitForExpectations:@[ getErrorCountExpectation, getWarningsCountExpectation ]
-                    timeout:0.5
-               enforceOrder:YES];
+- (void)testNumberOfIssuesLoggedNoDeadlock {
+  [self dispatchSyncNestedDispatchCount:100
+                                  queue:getGULLoggerCounterQueue()
+                                  block:^{
+                                    XCTAssertNoThrow(GULNumberOfErrorsLogged());
+                                    XCTAssertNoThrow(GULNumberOfWarningsLogged());
+                                  }];
 }
 
 // Helper functions.
 - (BOOL)logExists {
-  [self drainGULClientQueue];
+  [self drainQueue:getGULClientQueue()];
   NSString *correctMsg =
       [NSString stringWithFormat:@"%@[%@] %@", @"my service", kMessageCode, self.randomLogString];
   return [self messageWasLogged:correctMsg];
 }
 
-- (void)drainGULClientQueue {
+- (void)drainQueue:(dispatch_queue_t)queue {
   dispatch_semaphore_t workerSemaphore = dispatch_semaphore_create(0);
-  dispatch_async(getGULClientQueue(), ^{
+  dispatch_barrier_async(queue, ^{
     dispatch_semaphore_signal(workerSemaphore);
   });
   dispatch_semaphore_wait(workerSemaphore, DISPATCH_TIME_FOREVER);
@@ -272,5 +241,19 @@ - (BOOL)messageWasLogged:(NSString *)message {
 #pragma clang pop
 }
 
+- (void)dispatchSyncNestedDispatchCount:(NSInteger)count
+                                  queue:(dispatch_queue_t)queue
+                                  block:(dispatch_block_t)block {
+  if (count < 0) {
+    return;
+  }
+
+  dispatch_sync(queue, ^{
+    [self dispatchSyncNestedDispatchCount:count - 1 queue:queue block:block];
+    block();
+    NSLog(@"%@, depth: %ld", NSStringFromSelector(_cmd), (long)count);
+  });
+}
+
 @end
 #endif

GoogleUtilities/Logger/GULLogger.m

@@ -203,62 +203,79 @@ void GULLogBasic(GULLoggerLevel level,
 
 #undef GUL_MAKE_LOGGER
 
-#pragma mark - Number of errors and warnings
+#pragma mark - User defaults
 
-NSUserDefaults *getGULLoggerUsetDefaults(void) {
-  static NSUserDefaults *_userDefaults = nil;
-  static dispatch_once_t onceToken;
-  dispatch_once(&onceToken, ^{
-    _userDefaults = [[NSUserDefaults alloc] initWithSuiteName:@"GoogleUtilities.Logger.GULLogger"];
-  });
+// NSUserDefaults cannot be used due to a bug described in GULUserDefaults
+// GULUserDefaults cannot be used because GULLogger is a dependency for GULUserDefaults
+// We have to use C API deireclty here
 
-  return _userDefaults;
+CFStringRef getGULLoggerUsetDefaultsSuiteName(void) {
+  return (__bridge CFStringRef) @"GoogleUtilities.Logger.GULLogger";
 }
 
+NSInteger GULGetUserDefaultsIntegerForKey(NSString *key) {
+  id value = (__bridge_transfer id)CFPreferencesCopyAppValue((__bridge CFStringRef)key,
+                                                             getGULLoggerUsetDefaultsSuiteName());
+  if (![value isKindOfClass:[NSNumber class]]) {
+    return 0;
+  }
+
+  return [(NSNumber *)value integerValue];
+}
+
+void GULLoggerUserDefaultsSetIntegerForKey(NSInteger count, NSString *key) {
+  NSNumber *countNumber = @(count);
+  CFPreferencesSetAppValue((__bridge CFStringRef)key, (__bridge CFNumberRef)countNumber,
+                           getGULLoggerUsetDefaultsSuiteName());
+  CFPreferencesAppSynchronize(getGULLoggerUsetDefaultsSuiteName());
+}
+
+#pragma mark - Number of errors and warnings
+
 dispatch_queue_t getGULLoggerCounterQueue(void) {
   static dispatch_queue_t queue;
   static dispatch_once_t onceToken;
   dispatch_once(&onceToken, ^{
-    queue = dispatch_queue_create("GoogleUtilities.GULLogger.counterQueue", DISPATCH_QUEUE_SERIAL);
+    queue =
+        dispatch_queue_create("GoogleUtilities.GULLogger.counterQueue", DISPATCH_QUEUE_CONCURRENT);
     dispatch_set_target_queue(queue,
                               dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_BACKGROUND, 0));
   });
 
   return queue;
 }
 
-void GULAsyncGetUserDefaultsIntegerForKey(NSString *key, void (^completion)(NSInteger)) {
-  dispatch_async(getGULLoggerCounterQueue(), ^{
-    NSInteger count = [getGULLoggerUsetDefaults() integerForKey:key];
-    dispatch_async(dispatch_get_main_queue(), ^{
-      completion(count);
-    });
+NSInteger GULSyncGetUserDefaultsIntegerForKey(NSString *key) {
+  __block NSInteger integerValue = 0;
+  dispatch_sync(getGULLoggerCounterQueue(), ^{
+    integerValue = GULGetUserDefaultsIntegerForKey(key);
   });
+
+  return integerValue;
 }
 
-void GULNumberOfErrorsLogged(void (^completion)(NSInteger)) {
-  GULAsyncGetUserDefaultsIntegerForKey(kGULLoggerErrorCountKey, completion);
+NSInteger GULNumberOfErrorsLogged(void) {
+  return GULSyncGetUserDefaultsIntegerForKey(kGULLoggerErrorCountKey);
 }
 
-void GULNumberOfWarningsLogged(void (^completion)(NSInteger)) {
-  GULAsyncGetUserDefaultsIntegerForKey(kGULLoggerWarningCountKey, completion);
+NSInteger GULNumberOfWarningsLogged(void) {
+  return GULSyncGetUserDefaultsIntegerForKey(kGULLoggerWarningCountKey);
 }
 
 void GULResetNumberOfIssuesLogged(void) {
-  dispatch_async(getGULLoggerCounterQueue(), ^{
-    [getGULLoggerUsetDefaults() setInteger:0 forKey:kGULLoggerErrorCountKey];
-    [getGULLoggerUsetDefaults() setInteger:0 forKey:kGULLoggerWarningCountKey];
+  dispatch_barrier_async(getGULLoggerCounterQueue(), ^{
+    GULLoggerUserDefaultsSetIntegerForKey(0, kGULLoggerErrorCountKey);
+    GULLoggerUserDefaultsSetIntegerForKey(0, kGULLoggerWarningCountKey);
   });
 }
 
 void GULIncrementUserDefaultsIntegerForKey(NSString *key) {
-  NSUserDefaults *defaults = getGULLoggerUsetDefaults();
-  NSInteger errorCount = [defaults integerForKey:key];
-  [defaults setInteger:errorCount + 1 forKey:key];
+  NSInteger value = GULGetUserDefaultsIntegerForKey(key);
+  GULLoggerUserDefaultsSetIntegerForKey(value + 1, key);
 }
 
 void GULIncrementLogCountForLevel(GULLoggerLevel level) {
-  dispatch_async(getGULLoggerCounterQueue(), ^{
+  dispatch_barrier_async(getGULLoggerCounterQueue(), ^{
     if (level == GULLoggerLevelError) {
       GULIncrementUserDefaultsIntegerForKey(kGULLoggerErrorCountKey);
     } else if (level == GULLoggerLevelWarning) {

GoogleUtilities/Logger/Private/GULLogger.h

@@ -141,13 +141,15 @@ extern void GULLogDebug(GULLoggerService service,
 
 /**
  * Retrieve the number of errors that have been logged since the stat was last reset.
+ * Calling this method can be comparably expensive, so it should not be called from main thread.
  */
-extern void GULNumberOfErrorsLogged(void (^completion)(NSInteger));
+extern NSInteger GULNumberOfErrorsLogged(void);
 
 /**
  * Retrieve the number of warnings that have been logged since the stat was last reset.
+ * Calling this method can be comparably expensive, so it should not be called from main thread.
  */
-extern void GULNumberOfWarningsLogged(void (^completion)(NSInteger));
+extern NSInteger GULNumberOfWarningsLogged(void);
 
 /**
  * Reset number of errors and warnings that have been logged to 0.