From 0a54e7a10106d8b6b3579afc6edeeb66b3f67063 Mon Sep 17 00:00:00 2001 From: Byoungchan Lee Date: Mon, 6 Sep 2021 22:32:52 +0900 Subject: [PATCH] Verify that locks are handled correctly on RTCAudioSession MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL is for the same behavior as before [1], to emit the NSError when an application is not using the RTCAudioSession lock correctly. [1] https://webrtc-review.googlesource.com/c/src/+/207432 Bug: webrtc:13091 Change-Id: I031b0e963d33c92ce1af7a306edfa6be005e043d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/229461 Reviewed-by: Niels Moller Reviewed-by: Mirko Bonadei Reviewed-by: Kári Helgason Commit-Queue: Byoungchan Lee Cr-Commit-Position: refs/heads/main@{#35018} --- sdk/BUILD.gn | 3 + sdk/objc/components/audio/RTCAudioSession.mm | 69 ++++++++++++++++++++ sdk/objc/unittests/RTCAudioSessionTest.mm | 41 ++++++++++++ 3 files changed, 113 insertions(+) diff --git a/sdk/BUILD.gn b/sdk/BUILD.gn index 4b89f60ee6..90e245a18d 100644 --- a/sdk/BUILD.gn +++ b/sdk/BUILD.gn @@ -336,6 +336,8 @@ if (is_ios || is_mac) { "../rtc_base:rtc_base_approved", "../rtc_base/synchronization:mutex", ] + + absl_deps = [ "//third_party/abseil-cpp/absl/base:core_headers" ] } rtc_source_set("network_monitor_observer") { @@ -1196,6 +1198,7 @@ if (is_ios || is_mac) { "../modules/audio_processing:api", "../modules/video_coding:video_codec_interface", "../rtc_base:gunit_helpers", + "../rtc_base:rtc_event", "../rtc_base/system:unused", "../test:test_support", "//third_party/ocmock", diff --git a/sdk/objc/components/audio/RTCAudioSession.mm b/sdk/objc/components/audio/RTCAudioSession.mm index 5031e1542e..0d0db5aa5b 100644 --- a/sdk/objc/components/audio/RTCAudioSession.mm +++ b/sdk/objc/components/audio/RTCAudioSession.mm @@ -14,6 +14,7 @@ #include +#include "absl/base/attributes.h" #include "rtc_base/atomic_ops.h" #include "rtc_base/checks.h" #include "rtc_base/synchronization/mutex.h" @@ -21,11 +22,21 @@ #import "RTCAudioSessionConfiguration.h" #import "base/RTCLogging.h" +#if !defined(ABSL_HAVE_THREAD_LOCAL) +#error ABSL_HAVE_THREAD_LOCAL should be defined for MacOS / iOS Targets. +#endif + NSString *const kRTCAudioSessionErrorDomain = @"org.webrtc.RTC_OBJC_TYPE(RTCAudioSession)"; NSInteger const kRTCAudioSessionErrorLockRequired = -1; NSInteger const kRTCAudioSessionErrorConfiguration = -2; NSString * const kRTCAudioSessionOutputVolumeSelector = @"outputVolume"; +namespace { +// Since webrtc::Mutex is not a reentrant lock and cannot check if the mutex is locked, +// we need a separate variable to check that the mutex is locked in the RTCAudioSession. +ABSL_CONST_INIT thread_local bool mutex_locked = false; +} // namespace + @interface RTC_OBJC_TYPE (RTCAudioSession) () @property(nonatomic, readonly) std::vector<__weak id > delegates; @@ -229,10 +240,13 @@ NSString * const kRTCAudioSessionOutputVolumeSelector = @"outputVolume"; #pragma clang diagnostic ignored "-Wthread-safety-analysis" - (void)lockForConfiguration { + RTC_CHECK(!mutex_locked); _mutex.Lock(); + mutex_locked = true; } - (void)unlockForConfiguration { + mutex_locked = false; _mutex.Unlock(); } @@ -334,6 +348,9 @@ NSString * const kRTCAudioSessionOutputVolumeSelector = @"outputVolume"; - (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."); @@ -393,52 +410,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]; } @@ -559,6 +609,15 @@ 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 >)delegates { @synchronized(self) { // Note: this returns a copy. @@ -620,6 +679,16 @@ NSString * const kRTCAudioSessionOutputVolumeSelector = @"outputVolume"; } } +- (BOOL)checkLock:(NSError **)outError { + if (!mutex_locked) { + if (outError) { + *outError = [RTC_OBJC_TYPE(RTCAudioSession) lockError]; + } + return NO; + } + return YES; +} + - (BOOL)beginWebRTCSession:(NSError **)outError { if (outError) { *outError = nil; diff --git a/sdk/objc/unittests/RTCAudioSessionTest.mm b/sdk/objc/unittests/RTCAudioSessionTest.mm index e2c26634b0..9417ef1b68 100644 --- a/sdk/objc/unittests/RTCAudioSessionTest.mm +++ b/sdk/objc/unittests/RTCAudioSessionTest.mm @@ -13,6 +13,7 @@ #include +#include "rtc_base/event.h" #include "rtc_base/gunit.h" #import "components/audio/RTCAudioSession+Private.h" @@ -271,6 +272,41 @@ OCMLocation *OCMMakeLocation(id testCase, const char *fileCString, int line){ [mockAudioSession stopMocking]; } +- (void)testConfigureWebRTCSessionWithoutLocking { + NSError *error = nil; + + id mockAVAudioSession = OCMPartialMock([AVAudioSession sharedInstance]); + id mockAudioSession = OCMPartialMock([RTC_OBJC_TYPE(RTCAudioSession) sharedInstance]); + OCMStub([mockAudioSession session]).andReturn(mockAVAudioSession); + + RTC_OBJC_TYPE(RTCAudioSession) *audioSession = mockAudioSession; + + std::unique_ptr thread = rtc::Thread::Create(); + EXPECT_TRUE(thread); + EXPECT_TRUE(thread->Start()); + + rtc::Event waitLock; + rtc::Event waitCleanup; + constexpr int timeoutMs = 5000; + thread->PostTask(RTC_FROM_HERE, [audioSession, &waitLock, &waitCleanup] { + [audioSession lockForConfiguration]; + waitLock.Set(); + waitCleanup.Wait(timeoutMs); + [audioSession unlockForConfiguration]; + }); + + waitLock.Wait(timeoutMs); + [audioSession setCategory:AVAudioSessionCategoryPlayAndRecord withOptions:0 error:&error]; + EXPECT_TRUE(error != nil); + EXPECT_EQ(error.domain, kRTCAudioSessionErrorDomain); + EXPECT_EQ(error.code, kRTCAudioSessionErrorLockRequired); + waitCleanup.Set(); + thread->Stop(); + + [mockAVAudioSession stopMocking]; + [mockAudioSession stopMocking]; +} + - (void)testAudioVolumeDidNotify { MockAVAudioSession *mockAVAudioSession = [[MockAVAudioSession alloc] init]; RTC_OBJC_TYPE(RTCAudioSession) *session = @@ -329,6 +365,11 @@ TEST_F(AudioSessionTest, ConfigureWebRTCSession) { [test testConfigureWebRTCSession]; } +TEST_F(AudioSessionTest, ConfigureWebRTCSessionWithoutLocking) { + RTCAudioSessionTest *test = [[RTCAudioSessionTest alloc] init]; + [test testConfigureWebRTCSessionWithoutLocking]; +} + TEST_F(AudioSessionTest, AudioVolumeDidNotify) { RTCAudioSessionTest *test = [[RTCAudioSessionTest alloc] init]; [test testAudioVolumeDidNotify];