From edbae5e0ac5be950a8c4372d420c8229bdc7f161 Mon Sep 17 00:00:00 2001 From: denicija Date: Fri, 30 Sep 2016 00:21:11 -0700 Subject: [PATCH] Remove Crit::Scope lock by using atomic bool property. The clang static analyzer seems unable to resolve cpp locks in ObjC code. As of current time, the clang analyzer has known limitations documented http://clang.llvm.org/docs/ThreadSafetyAnalysis.html#known-limitations. From the documentation: "The analysis currently does not do any checking inside constructors or destructors. In other words, every constructor and destructor is treated as if it was annotated with NO_THREAD_SAFETY_ANALYSIS." This is 'probably' why the analyzer is unable to resolve the lock when used in ObjC land (the cpp works fine). The lock can be removed by using atomic property instead. It's not on performance critical path and we expect updates on just one queue and reads from others. That's why the thread assurance atomic properties bring is enough. The CL removes rtc_sdk_peerconnection_objc_warnings_config as well as it's no longer needed. BUG=webrtc:6308 Review-Url: https://codereview.webrtc.org/2372513004 Cr-Commit-Position: refs/heads/master@{#14450} --- webrtc/sdk/BUILD.gn | 10 ---------- .../Classes/avfoundationvideocapturer.mm | 15 ++++----------- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/webrtc/sdk/BUILD.gn b/webrtc/sdk/BUILD.gn index 0fd8043927..4cceb8e1a6 100644 --- a/webrtc/sdk/BUILD.gn +++ b/webrtc/sdk/BUILD.gn @@ -68,15 +68,6 @@ if (is_ios || (is_mac && mac_deployment_target == "10.7")) { } } - config("rtc_sdk_peerconnection_objc_warnings_config") { - if (is_clang) { - # TODO(tkchin): Make rtc_sdk_peerconnection_objc compile with the standard - # set of warnings. - # See https://bugs.chromium.org/p/webrtc/issues/detail?id=6308 - cflags = [ "-Wno-thread-safety-analysis" ] - } - } - rtc_static_library("rtc_sdk_peerconnection_objc") { sources = [ "objc/Framework/Classes/RTCAVFoundationVideoSource+Private.h", @@ -194,7 +185,6 @@ if (is_ios || (is_mac && mac_deployment_target == "10.7")) { } configs += [ - ":rtc_sdk_peerconnection_objc_warnings_config", "..:common_objc", "//build/config/compiler:enable_arc", ] diff --git a/webrtc/sdk/objc/Framework/Classes/avfoundationvideocapturer.mm b/webrtc/sdk/objc/Framework/Classes/avfoundationvideocapturer.mm index c71c9fdbd3..052cdf8057 100644 --- a/webrtc/sdk/objc/Framework/Classes/avfoundationvideocapturer.mm +++ b/webrtc/sdk/objc/Framework/Classes/avfoundationvideocapturer.mm @@ -78,7 +78,7 @@ static NSString *GetSessionPresetForVideoFormat( @property(nonatomic, readonly) dispatch_queue_t frameQueue; @property(nonatomic, readonly) BOOL canUseBackCamera; @property(nonatomic, assign) BOOL useBackCamera; // Defaults to NO. -@property(nonatomic, assign) BOOL isRunning; // Whether the capture session is running. +@property(atomic, assign) BOOL isRunning; // Whether the capture session is running. @property(atomic, assign) BOOL hasStarted; // Whether we have an unmatched start. // We keep a pointer back to AVFoundationVideoCapturer to make callbacks on it @@ -111,6 +111,8 @@ static NSString *GetSessionPresetForVideoFormat( @synthesize captureSession = _captureSession; @synthesize frameQueue = _frameQueue; @synthesize useBackCamera = _useBackCamera; + +@synthesize isRunning = _isRunning; @synthesize hasStarted = _hasStarted; // This is called from the thread that creates the video source, which is likely @@ -217,16 +219,6 @@ static NSString *GetSessionPresetForVideoFormat( } } -- (BOOL)isRunning { - rtc::CritScope cs(&_crit); - return _isRunning; -} - -- (void)setIsRunning:(BOOL)isRunning { - rtc::CritScope cs(&_crit); - _isRunning = isRunning; -} - // Called from WebRTC thread. - (void)start { if (self.hasStarted) { @@ -355,6 +347,7 @@ static NSString *GetSessionPresetForVideoFormat( - (void)handleCaptureSessionDidStartRunning:(NSNotification *)notification { RTCLog(@"Capture session started."); + self.isRunning = YES; [RTCDispatcher dispatchAsyncOnType:RTCDispatcherTypeCaptureSession block:^{