Enhance RTCUIApplicationStatusObserver thread safety.

webrtc/sdk/objc/Framework/Classes/Common/RTCUIApplicationStatusObserver.m

 /*
  *  Copyright 2016 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "RTCUIApplicationStatusObserver.h"
 
 #if defined(WEBRTC_IOS)
 
 #import <UIKit/UIKit.h>
 
 #include "webrtc/rtc_base/checks.h"
 
 @interface RTCUIApplicationStatusObserver ()
 
 @property(nonatomic, assign) BOOL initialized;
 @property(nonatomic, assign) UIApplicationState state;

[tkchin_webrtc, 2017/09/13 18:32:02]

State is read and written to from multiple threads. It should be atomic, and
only accessed via property accessor instead of directly through _state. 

[andersc, 2017/09/14 08:08:45]

Acknowledged.

 
 @end
 
 @implementation RTCUIApplicationStatusObserver {
   BOOL _initialized;
   dispatch_block_t _initializeBlock;
+  dispatch_semaphore_t _waitForInitializeSemaphore;
   UIApplicationState _state;
 
   id<NSObject> _activeObserver;
   id<NSObject> _backgroundObserver;
 }
 
 @synthesize initialized = _initialized;
 @synthesize state = _state;
 
 + (instancetype)sharedInstance {
   static id sharedInstance;
   static dispatch_once_t onceToken;
   dispatch_once(&onceToken, ^{
     sharedInstance = [[self alloc] init];
   });
 
   return sharedInstance;
 }
 
+// Method to make sure observers are added and the initialization block is
+// scheduled to run on the main queue.
++ (void)prepareForUse {

[tkchin_webrtc, 2017/09/13 18:32:02]

This class feels more complex than it needs to be.
I suspect that if we constructed multiple instances instead of a singleton,
created the instance in encoder / decoder ctors, provided an isReady method,
ignored frames until main queue initialization is complete, we'd have the same
thing working. I'd guess there'd be sufficient time between encoder / decoder
ctor and the first encode/decode method call to complete init in most cases
because it will take time for capture to begin.

[andersc, 2017/09/14 08:08:45]

I agree, and I mentioned a solution similar to that in b/65558688#comment4

The reason I did it this way was that it is a logically smaller change than from
what we had before, and I'm not confident that returning OK from the
encoder/decoder without performing the work doesn't impact any other record
keeping made by WebRTC. But it should be easy to try out and see if it works as
expected. Let me know if you think we should spend time on trying such a
solution out instead.

+  __unused RTCUIApplicationStatusObserver *observer = [self sharedInstance];
+}
+
 - (id)init {
   if (self = [super init]) {
     NSNotificationCenter *center = [NSNotificationCenter defaultCenter];
     __weak RTCUIApplicationStatusObserver *weakSelf = self;
     _activeObserver = [center addObserverForName:UIApplicationDidBecomeActiveNotification
                                           object:nil
                                            queue:[NSOperationQueue mainQueue]
                                       usingBlock:^(NSNotification *note) {
                                         weakSelf.state =
                                             [UIApplication sharedApplication].applicationState;
                                       }];
 
     _backgroundObserver = [center addObserverForName:UIApplicationDidEnterBackgroundNotification
                                               object:nil
                                                queue:[NSOperationQueue mainQueue]
                                           usingBlock:^(NSNotification *note) {
                                             weakSelf.state =
                                                 [UIApplication sharedApplication].applicationState;
                                           }];
 
+    _waitForInitializeSemaphore = dispatch_semaphore_create(1);
     _initialized = NO;
     _initializeBlock = dispatch_block_create(DISPATCH_BLOCK_INHERIT_QOS_CLASS, ^{
       weakSelf.state = [UIApplication sharedApplication].applicationState;
       weakSelf.initialized = YES;
     });
 
     dispatch_async(dispatch_get_main_queue(), _initializeBlock);
   }
 
   return self;
 }
 
 - (void)dealloc {
   NSNotificationCenter *center = [NSNotificationCenter defaultCenter];
   [center removeObserver:_activeObserver];
   [center removeObserver:_backgroundObserver];
 }
 
 - (BOOL)isApplicationActive {
+  // NOTE: The function `dispatch_block_wait` can only legally be called once.
+  // Because of this, if several threads call the `isApplicationActive` method before
+  // the `_initializeBlock` has been executed, instead of multiple threads calling
+  // `dispatch_block_wait`, the other threads need to wait for the first waiting thread
+  // instead.
   if (!_initialized) {
-    long ret = dispatch_block_wait(_initializeBlock,
-                                   dispatch_time(DISPATCH_TIME_NOW, 10.0 * NSEC_PER_SEC));
-    RTC_DCHECK_EQ(ret, 0);
+    dispatch_semaphore_wait(_waitForInitializeSemaphore, DISPATCH_TIME_FOREVER);
+    if (!_initialized) {
+      long ret = dispatch_block_wait(_initializeBlock,
+                                     dispatch_time(DISPATCH_TIME_NOW, 10.0 * NSEC_PER_SEC));

[tkchin_webrtc, 2017/09/13 18:32:02]

What's the benefit of setting a short timeout here? It seems to introduce bad
behavior if the timeout isn't met, and it seems hard to guarantee that this
number works for all iOS hardware. DISPATCH_TIME_FOREVER seems more appropriate.

[andersc, 2017/09/14 08:08:46]

10 seconds was picked to be a long timeout. A test deadlocked (because the main
queue was blocked on something else), so added the DCHECK to make it easier to
debug similar issues in the future, rather than the test just hanging.

+      RTC_DCHECK_EQ(ret, 0);
+    }
+    dispatch_semaphore_signal(_waitForInitializeSemaphore);
   }
   return _state == UIApplicationStateActive;
 }
 
 @end
 
 #endif  // WEBRTC_IOS