diff --git a/GoogleUtilities/Example/Tests/Logger/GULLoggerTest.m b/GoogleUtilities/Example/Tests/Logger/GULLoggerTest.m index a9e0486cf94..35109e71518 100644 --- a/GoogleUtilities/Example/Tests/Logger/GULLoggerTest.m +++ b/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 diff --git a/GoogleUtilities/Logger/GULLogger.m b/GoogleUtilities/Logger/GULLogger.m index 4e8feafce8f..7a88af8cf5e 100644 --- a/GoogleUtilities/Logger/GULLogger.m +++ b/GoogleUtilities/Logger/GULLogger.m @@ -203,23 +203,41 @@ 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)); }); @@ -227,38 +245,37 @@ dispatch_queue_t getGULLoggerCounterQueue(void) { 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) { diff --git a/GoogleUtilities/Logger/Private/GULLogger.h b/GoogleUtilities/Logger/Private/GULLogger.h index 453de4bd955..167f24c4caa 100644 --- a/GoogleUtilities/Logger/Private/GULLogger.h +++ b/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.