Revert "Replace RecursiveCriticalSection with Mutex in RTCAudioSession."

This reverts commit f8da43d179043f1df2e1c3e2c49494bc23f4ec28.

Reason for revert: Appears to break downstream app.

Original change's description:
> Replace RecursiveCriticalSection with Mutex in RTCAudioSession.
>
> Bug: webrtc:11567
> Change-Id: I2a2ddbce57d070d6cbad5a64defb4c27be77a665
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/206472
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Kári Helgason <kthelgason@webrtc.org>
> Commit-Queue: Niels Moller <nisse@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#33259}

TBR=nisse@webrtc.org,kthelgason@webrtc.org,hta@webrtc.org,handellm@webrtc.org

Change-Id: Id9a97068722c7c72fc5d21102298249fd7a7cd9a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:11567
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/207431
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33268}

sdk/BUILD.gn

@@ -334,7 +334,6 @@ if (is_ios || is_mac) {
           "../rtc_base",
           "../rtc_base:checks",
           "../rtc_base:rtc_base_approved",
-          "../rtc_base/synchronization:mutex",
         ]
       }
 

sdk/objc/components/audio/RTCAudioSession+Configuration.mm

@@ -43,6 +43,9 @@
   if (outError) {
     *outError = nil;
   }
+  if (![self checkLock:outError]) {
+    return NO;
+  }
 
   // Provide an error even if there isn't one so we can log it. We will not
   // return immediately on error in this function and instead try to set

sdk/objc/components/audio/RTCAudioSession+Private.h

@@ -35,6 +35,8 @@ NS_ASSUME_NONNULL_BEGIN
  */
 @property(nonatomic, assign) BOOL isInterrupted;
 
+- (BOOL)checkLock:(NSError **)outError;
+
 /** Adds the delegate to the list of delegates, and places it at the front of
  *  the list. This delegate will be notified before other delegates of
  *  audio events.

sdk/objc/components/audio/RTCAudioSession.mm

@@ -16,7 +16,7 @@
 
 #include "rtc_base/atomic_ops.h"
 #include "rtc_base/checks.h"
-#include "rtc_base/synchronization/mutex.h"
+#include "rtc_base/deprecated/recursive_critical_section.h"
 
 #import "RTCAudioSessionConfiguration.h"
 #import "base/RTCLogging.h"
@@ -35,9 +35,10 @@ NSString * const kRTCAudioSessionOutputVolumeSelector = @"outputVolume";
 // TODO(tkchin): Consider more granular locking. We're not expecting a lot of
 // lock contention so coarse locks should be fine for now.
 @implementation RTC_OBJC_TYPE (RTCAudioSession) {
-  webrtc::Mutex _mutex;
+  rtc::RecursiveCriticalSection _crit;
   AVAudioSession *_session;
   volatile int _activationCount;
+  volatile int _lockRecursionCount;
   volatile int _webRTCSessionCount;
   BOOL _isActive;
   BOOL _useManualAudio;
@@ -150,6 +151,10 @@ NSString * const kRTCAudioSessionOutputVolumeSelector = @"outputVolume";
   }
 }
 
+- (BOOL)isLocked {
+  return _lockRecursionCount > 0;
+}
+
 - (void)setUseManualAudio:(BOOL)useManualAudio {
   @synchronized(self) {
     if (_useManualAudio == useManualAudio) {
@@ -229,11 +234,20 @@ NSString * const kRTCAudioSessionOutputVolumeSelector = @"outputVolume";
 #pragma clang diagnostic ignored "-Wthread-safety-analysis"
 
 - (void)lockForConfiguration {
-  _mutex.Lock();
+  _crit.Enter();
+  rtc::AtomicOps::Increment(&_lockRecursionCount);
 }
 
 - (void)unlockForConfiguration {
-  _mutex.Unlock();
+  // Don't let threads other than the one that called lockForConfiguration
+  // unlock.
+  if (_crit.TryEnter()) {
+    rtc::AtomicOps::Decrement(&_lockRecursionCount);
+    // One unlock for the tryLock, and another one to actually unlock. If this
+    // was called without anyone calling lock, we will hit an assertion.
+    _crit.Leave();
+    _crit.Leave();
+  }
 }
 
 #pragma clang diagnostic pop
@@ -332,8 +346,13 @@ NSString * const kRTCAudioSessionOutputVolumeSelector = @"outputVolume";
   return self.session.preferredIOBufferDuration;
 }
 
+// TODO(tkchin): Simplify the amount of locking happening here. Likely that we
+// can just do atomic increments / decrements.
 - (BOOL)setActive:(BOOL)active
             error:(NSError **)outError {
+  if (![self checkLock:outError]) {
+    return NO;
+  }
   int activationCount = _activationCount;
   if (!active && activationCount == 0) {
     RTCLogWarning(@"Attempting to deactivate without prior activation.");
@@ -390,52 +409,85 @@ NSString * const kRTCAudioSessionOutputVolumeSelector = @"outputVolume";
 - (BOOL)setCategory:(NSString *)category
         withOptions:(AVAudioSessionCategoryOptions)options
               error:(NSError **)outError {
+  if (![self checkLock:outError]) {
+    return NO;
+  }
   return [self.session setCategory:category withOptions:options error:outError];
 }
 
 - (BOOL)setMode:(NSString *)mode error:(NSError **)outError {
+  if (![self checkLock:outError]) {
+    return NO;
+  }
   return [self.session setMode:mode error:outError];
 }
 
 - (BOOL)setInputGain:(float)gain error:(NSError **)outError {
+  if (![self checkLock:outError]) {
+    return NO;
+  }
   return [self.session setInputGain:gain error:outError];
 }
 
 - (BOOL)setPreferredSampleRate:(double)sampleRate error:(NSError **)outError {
+  if (![self checkLock:outError]) {
+    return NO;
+  }
   return [self.session setPreferredSampleRate:sampleRate error:outError];
 }
 
 - (BOOL)setPreferredIOBufferDuration:(NSTimeInterval)duration
                                error:(NSError **)outError {
+  if (![self checkLock:outError]) {
+    return NO;
+  }
   return [self.session setPreferredIOBufferDuration:duration error:outError];
 }
 
 - (BOOL)setPreferredInputNumberOfChannels:(NSInteger)count
                                     error:(NSError **)outError {
+  if (![self checkLock:outError]) {
+    return NO;
+  }
   return [self.session setPreferredInputNumberOfChannels:count error:outError];
 }
 - (BOOL)setPreferredOutputNumberOfChannels:(NSInteger)count
                                      error:(NSError **)outError {
+  if (![self checkLock:outError]) {
+    return NO;
+  }
   return [self.session setPreferredOutputNumberOfChannels:count error:outError];
 }
 
 - (BOOL)overrideOutputAudioPort:(AVAudioSessionPortOverride)portOverride
                           error:(NSError **)outError {
+  if (![self checkLock:outError]) {
+    return NO;
+  }
   return [self.session overrideOutputAudioPort:portOverride error:outError];
 }
 
 - (BOOL)setPreferredInput:(AVAudioSessionPortDescription *)inPort
                     error:(NSError **)outError {
+  if (![self checkLock:outError]) {
+    return NO;
+  }
   return [self.session setPreferredInput:inPort error:outError];
 }
 
 - (BOOL)setInputDataSource:(AVAudioSessionDataSourceDescription *)dataSource
                      error:(NSError **)outError {
+  if (![self checkLock:outError]) {
+    return NO;
+  }
   return [self.session setInputDataSource:dataSource error:outError];
 }
 
 - (BOOL)setOutputDataSource:(AVAudioSessionDataSourceDescription *)dataSource
                       error:(NSError **)outError {
+  if (![self checkLock:outError]) {
+    return NO;
+  }
   return [self.session setOutputDataSource:dataSource error:outError];
 }
 
@@ -556,6 +608,18 @@ NSString * const kRTCAudioSessionOutputVolumeSelector = @"outputVolume";
 
 #pragma mark - Private
 
++ (NSError *)lockError {
+  NSDictionary *userInfo = @{
+    NSLocalizedDescriptionKey:
+        @"Must call lockForConfiguration before calling this method."
+  };
+  NSError *error =
+      [[NSError alloc] initWithDomain:kRTCAudioSessionErrorDomain
+                                 code:kRTCAudioSessionErrorLockRequired
+                             userInfo:userInfo];
+  return error;
+}
+
 - (std::vector<__weak id<RTC_OBJC_TYPE(RTCAudioSessionDelegate)> >)delegates {
   @synchronized(self) {
     // Note: this returns a copy.
@@ -617,10 +681,25 @@ NSString * const kRTCAudioSessionOutputVolumeSelector = @"outputVolume";
   }
 }
 
+- (BOOL)checkLock:(NSError **)outError {
+  // Check ivar instead of trying to acquire lock so that we won't accidentally
+  // acquire lock if it hasn't already been called.
+  if (!self.isLocked) {
+    if (outError) {
+      *outError = [RTC_OBJC_TYPE(RTCAudioSession) lockError];
+    }
+    return NO;
+  }
+  return YES;
+}
+
 - (BOOL)beginWebRTCSession:(NSError **)outError {
   if (outError) {
     *outError = nil;
   }
+  if (![self checkLock:outError]) {
+    return NO;
+  }
   rtc::AtomicOps::Increment(&_webRTCSessionCount);
   [self notifyDidStartPlayOrRecord];
   return YES;
@@ -630,6 +709,9 @@ NSString * const kRTCAudioSessionOutputVolumeSelector = @"outputVolume";
   if (outError) {
     *outError = nil;
   }
+  if (![self checkLock:outError]) {
+    return NO;
+  }
   rtc::AtomicOps::Decrement(&_webRTCSessionCount);
   [self notifyDidStopPlayOrRecord];
   return YES;
@@ -639,6 +721,9 @@ NSString * const kRTCAudioSessionOutputVolumeSelector = @"outputVolume";
   if (outError) {
     *outError = nil;
   }
+  if (![self checkLock:outError]) {
+    return NO;
+  }
   RTCLog(@"Configuring audio session for WebRTC.");
 
   // Configure the AVAudioSession and activate it.
@@ -699,6 +784,9 @@ NSString * const kRTCAudioSessionOutputVolumeSelector = @"outputVolume";
   if (outError) {
     *outError = nil;
   }
+  if (![self checkLock:outError]) {
+    return NO;
+  }
   RTCLog(@"Unconfiguring audio session for WebRTC.");
   [self setActive:NO error:outError];
 

sdk/objc/unittests/RTCAudioSessionTest.mm

@@ -113,10 +113,26 @@
 
 @interface RTCAudioSessionTest : NSObject
 
+- (void)testLockForConfiguration;
+
 @end
 
 @implementation RTCAudioSessionTest
 
+- (void)testLockForConfiguration {
+  RTC_OBJC_TYPE(RTCAudioSession) *session = [RTC_OBJC_TYPE(RTCAudioSession) sharedInstance];
+
+  for (size_t i = 0; i < 2; i++) {
+    [session lockForConfiguration];
+    EXPECT_TRUE(session.isLocked);
+  }
+  for (size_t i = 0; i < 2; i++) {
+    EXPECT_TRUE(session.isLocked);
+    [session unlockForConfiguration];
+  }
+  EXPECT_FALSE(session.isLocked);
+}
+
 - (void)testAddAndRemoveDelegates {
   RTC_OBJC_TYPE(RTCAudioSession) *session = [RTC_OBJC_TYPE(RTCAudioSession) sharedInstance];
   NSMutableArray *delegates = [NSMutableArray array];
@@ -248,6 +264,7 @@ OCMLocation *OCMMakeLocation(id testCase, const char *fileCString, int line){
   RTC_OBJC_TYPE(RTCAudioSession) *audioSession = mockAudioSession;
   EXPECT_EQ(0, audioSession.activationCount);
   [audioSession lockForConfiguration];
+  EXPECT_TRUE([audioSession checkLock:nil]);
   // configureWebRTCSession is forced to fail in the above mock interface,
   // so activationCount should remain 0
   OCMExpect([[mockAVAudioSession ignoringNonObjectArgs] setActive:YES
@@ -299,6 +316,11 @@ class AudioSessionTest : public ::testing::Test {
   }
 };
 
+TEST_F(AudioSessionTest, LockForConfiguration) {
+  RTCAudioSessionTest *test = [[RTCAudioSessionTest alloc] init];
+  [test testLockForConfiguration];
+}
+
 TEST_F(AudioSessionTest, AddAndRemoveDelegates) {
   RTCAudioSessionTest *test = [[RTCAudioSessionTest alloc] init];
   [test testAddAndRemoveDelegates];