From fe19699a207e5ac8a655c69e2e8a83da2f4dd78e Mon Sep 17 00:00:00 2001 From: "tommi@webrtc.org" Date: Sat, 7 Feb 2015 22:35:54 +0000 Subject: [PATCH] Revert 8260 "Base RWLockWrapper on rtc::SharedExclusiveLock." Unfortunately this caused channel teardown to hang. More details in email(s). > Base RWLockWrapper on rtc::SharedExclusiveLock. > > Also moves rtc::Event and rtc::SharedExclusiveLock to rtc_base_approved. > > R=tommi@webrtc.org > BUG= > > Review URL: https://webrtc-codereview.appspot.com/38889004 TBR=pbos@webrtc.org Review URL: https://webrtc-codereview.appspot.com/34999005 Cr-Commit-Position: refs/heads/master@{#8284} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8284 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/base/BUILD.gn | 8 +- webrtc/base/base.gyp | 8 +- webrtc/base/event.cc | 8 +- webrtc/base/event.h | 1 + webrtc/system_wrappers/BUILD.gn | 6 ++ .../interface/rw_lock_wrapper.h | 6 ++ webrtc/system_wrappers/source/rw_lock.cc | 40 ++++---- .../system_wrappers/source/rw_lock_generic.cc | 77 +++++++++++++++ .../system_wrappers/source/rw_lock_generic.h | 46 +++++++++ .../system_wrappers/source/rw_lock_posix.cc | 51 ++++++++++ webrtc/system_wrappers/source/rw_lock_posix.h | 41 ++++++++ webrtc/system_wrappers/source/rw_lock_win.cc | 97 +++++++++++++++++++ webrtc/system_wrappers/source/rw_lock_win.h | 40 ++++++++ webrtc/system_wrappers/system_wrappers.gyp | 6 ++ 14 files changed, 400 insertions(+), 35 deletions(-) create mode 100644 webrtc/system_wrappers/source/rw_lock_generic.cc create mode 100644 webrtc/system_wrappers/source/rw_lock_generic.h create mode 100644 webrtc/system_wrappers/source/rw_lock_posix.cc create mode 100644 webrtc/system_wrappers/source/rw_lock_posix.h create mode 100644 webrtc/system_wrappers/source/rw_lock_win.cc create mode 100644 webrtc/system_wrappers/source/rw_lock_win.h diff --git a/webrtc/base/BUILD.gn b/webrtc/base/BUILD.gn index d62993add..e758d868d 100644 --- a/webrtc/base/BUILD.gn +++ b/webrtc/base/BUILD.gn @@ -106,8 +106,6 @@ static_library("rtc_base_approved") { sources = [ "checks.cc", "checks.h", - "event.cc", - "event.h", "exp_filter.cc", "exp_filter.h", "md5.cc", @@ -117,8 +115,6 @@ static_library("rtc_base_approved") { "platform_file.h", "safe_conversions.h", "safe_conversions_impl.h", - "sharedexclusivelock.cc", - "sharedexclusivelock.h", "stringencode.cc", "stringencode.h", "stringutils.cc", @@ -187,6 +183,8 @@ static_library("rtc_base") { "cryptstring.h", "diskcache.cc", "diskcache.h", + "event.cc", + "event.h", "fileutils.cc", "fileutils.h", "firewallsocketserver.cc", @@ -362,6 +360,8 @@ static_library("rtc_base") { "scopedptrcollection.h", "scoped_ref_ptr.h", "sec_buffer.h", + "sharedexclusivelock.cc", + "sharedexclusivelock.h", "sslconfig.h", "sslroots.h", "stringdigest.h", diff --git a/webrtc/base/base.gyp b/webrtc/base/base.gyp index 0d1cb056f..74ba76ae3 100644 --- a/webrtc/base/base.gyp +++ b/webrtc/base/base.gyp @@ -31,8 +31,6 @@ 'sources': [ 'checks.cc', 'checks.h', - 'event.cc', - 'event.h', 'exp_filter.cc', 'exp_filter.h', 'md5.cc', @@ -42,8 +40,6 @@ 'platform_file.h', 'safe_conversions.h', 'safe_conversions_impl.h', - 'sharedexclusivelock.cc', - 'sharedexclusivelock.h', 'stringencode.cc', 'stringencode.h', 'stringutils.cc', @@ -117,6 +113,8 @@ 'diskcache.h', 'diskcache_win32.cc', 'diskcache_win32.h', + 'event.cc', + 'event.h', 'filelock.cc', 'filelock.h', 'fileutils.cc', @@ -229,6 +227,8 @@ 'sha1.cc', 'sha1.h', 'sha1digest.h', + 'sharedexclusivelock.cc', + 'sharedexclusivelock.h', 'signalthread.cc', 'signalthread.h', 'sigslot.h', diff --git a/webrtc/base/event.cc b/webrtc/base/event.cc index 606d6ad49..393142ea2 100644 --- a/webrtc/base/event.cc +++ b/webrtc/base/event.cc @@ -10,8 +10,6 @@ #include "webrtc/base/event.h" -#include "webrtc/base/checks.h" - #if defined(WEBRTC_WIN) #include #elif defined(WEBRTC_POSIX) @@ -33,7 +31,7 @@ Event::Event(bool manual_reset, bool initially_signaled) is_manual_reset_, is_initially_signaled_, NULL); // Name. - CHECK(event_handle_ != NULL); + ASSERT(event_handle_ != NULL); } Event::~Event() { @@ -58,8 +56,8 @@ bool Event::Wait(int cms) { Event::Event(bool manual_reset, bool initially_signaled) : is_manual_reset_(manual_reset), event_status_(initially_signaled) { - CHECK(pthread_mutex_init(&event_mutex_, NULL) == 0); - CHECK(pthread_cond_init(&event_cond_, NULL) == 0); + VERIFY(pthread_mutex_init(&event_mutex_, NULL) == 0); + VERIFY(pthread_cond_init(&event_cond_, NULL) == 0); } Event::~Event() { diff --git a/webrtc/base/event.h b/webrtc/base/event.h index 7a6209e3a..f2691a2f8 100644 --- a/webrtc/base/event.h +++ b/webrtc/base/event.h @@ -20,6 +20,7 @@ #endif #include "webrtc/base/basictypes.h" +#include "webrtc/base/common.h" namespace rtc { diff --git a/webrtc/system_wrappers/BUILD.gn b/webrtc/system_wrappers/BUILD.gn index d87ae0a7d..c4f1da5ef 100644 --- a/webrtc/system_wrappers/BUILD.gn +++ b/webrtc/system_wrappers/BUILD.gn @@ -76,6 +76,12 @@ static_library("system_wrappers") { "source/logging.cc", "source/rtp_to_ntp.cc", "source/rw_lock.cc", + "source/rw_lock_generic.cc", + "source/rw_lock_generic.h", + "source/rw_lock_posix.cc", + "source/rw_lock_posix.h", + "source/rw_lock_win.cc", + "source/rw_lock_win.h", "source/set_thread_name_win.h", "source/sleep.cc", "source/sort.cc", diff --git a/webrtc/system_wrappers/interface/rw_lock_wrapper.h b/webrtc/system_wrappers/interface/rw_lock_wrapper.h index 94c2c9774..dbe6d6c7c 100644 --- a/webrtc/system_wrappers/interface/rw_lock_wrapper.h +++ b/webrtc/system_wrappers/interface/rw_lock_wrapper.h @@ -13,6 +13,10 @@ #include "webrtc/base/thread_annotations.h" +// Note, Windows pre-Vista version of RW locks are not supported natively. For +// these OSs regular critical sections have been used to approximate RW lock +// functionality and will therefore have worse performance. + namespace webrtc { class LOCKABLE RWLockWrapper { @@ -27,6 +31,8 @@ class LOCKABLE RWLockWrapper { virtual void ReleaseLockShared() UNLOCK_FUNCTION() = 0; }; +// RAII extensions of the RW lock. Prevents Acquire/Release missmatches and +// provides more compact locking syntax. class SCOPED_LOCKABLE ReadLockScoped { public: ReadLockScoped(RWLockWrapper& rw_lock) SHARED_LOCK_FUNCTION(rw_lock) diff --git a/webrtc/system_wrappers/source/rw_lock.cc b/webrtc/system_wrappers/source/rw_lock.cc index ae2a6b75d..8b76eb861 100644 --- a/webrtc/system_wrappers/source/rw_lock.cc +++ b/webrtc/system_wrappers/source/rw_lock.cc @@ -10,32 +10,28 @@ #include "webrtc/system_wrappers/interface/rw_lock_wrapper.h" -#include "webrtc/base/sharedexclusivelock.h" -#include "webrtc/base/thread_annotations.h" +#include + +#if defined(_WIN32) +#include "webrtc/system_wrappers/source/rw_lock_generic.h" +#include "webrtc/system_wrappers/source/rw_lock_win.h" +#else +#include "webrtc/system_wrappers/source/rw_lock_posix.h" +#endif namespace webrtc { -class LOCKABLE RwLock : public RWLockWrapper { - virtual void AcquireLockExclusive() override EXCLUSIVE_LOCK_FUNCTION() { - lock_.LockExclusive(); - } - virtual void ReleaseLockExclusive() override UNLOCK_FUNCTION() { - lock_.UnlockExclusive(); - } - - virtual void AcquireLockShared() override SHARED_LOCK_FUNCTION() { - lock_.LockShared(); - } - virtual void ReleaseLockShared() override UNLOCK_FUNCTION() { - lock_.UnlockShared(); - } - - private: - rtc::SharedExclusiveLock lock_; -}; - RWLockWrapper* RWLockWrapper::CreateRWLock() { - return new RwLock(); +#ifdef _WIN32 + // Native implementation is faster, so use that if available. + RWLockWrapper* lock = RWLockWin::Create(); + if (lock) { + return lock; + } + return new RWLockGeneric(); +#else + return RWLockPosix::Create(); +#endif } } // namespace webrtc diff --git a/webrtc/system_wrappers/source/rw_lock_generic.cc b/webrtc/system_wrappers/source/rw_lock_generic.cc new file mode 100644 index 000000000..0ca951874 --- /dev/null +++ b/webrtc/system_wrappers/source/rw_lock_generic.cc @@ -0,0 +1,77 @@ +/* + * Copyright (c) 2011 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 "webrtc/system_wrappers/source/rw_lock_generic.h" + +#include "webrtc/system_wrappers/interface/condition_variable_wrapper.h" +#include "webrtc/system_wrappers/interface/critical_section_wrapper.h" + +namespace webrtc { + +RWLockGeneric::RWLockGeneric() + : readers_active_(0), + writer_active_(false), + readers_waiting_(0), + writers_waiting_(0) { + critical_section_ = CriticalSectionWrapper::CreateCriticalSection(); + read_condition_ = ConditionVariableWrapper::CreateConditionVariable(); + write_condition_ = ConditionVariableWrapper::CreateConditionVariable(); +} + +RWLockGeneric::~RWLockGeneric() { + delete write_condition_; + delete read_condition_; + delete critical_section_; +} + +void RWLockGeneric::AcquireLockExclusive() { + CriticalSectionScoped cs(critical_section_); + if (writer_active_ || readers_active_ > 0) { + ++writers_waiting_; + while (writer_active_ || readers_active_ > 0) { + write_condition_->SleepCS(*critical_section_); + } + --writers_waiting_; + } + writer_active_ = true; +} + +void RWLockGeneric::ReleaseLockExclusive() { + CriticalSectionScoped cs(critical_section_); + writer_active_ = false; + if (writers_waiting_ > 0) { + write_condition_->Wake(); + } else if (readers_waiting_ > 0) { + read_condition_->WakeAll(); + } +} + +void RWLockGeneric::AcquireLockShared() { + CriticalSectionScoped cs(critical_section_); + if (writer_active_ || writers_waiting_ > 0) { + ++readers_waiting_; + + while (writer_active_ || writers_waiting_ > 0) { + read_condition_->SleepCS(*critical_section_); + } + --readers_waiting_; + } + ++readers_active_; +} + +void RWLockGeneric::ReleaseLockShared() { + CriticalSectionScoped cs(critical_section_); + --readers_active_; + if (readers_active_ == 0 && writers_waiting_ > 0) { + write_condition_->Wake(); + } +} + +} // namespace webrtc diff --git a/webrtc/system_wrappers/source/rw_lock_generic.h b/webrtc/system_wrappers/source/rw_lock_generic.h new file mode 100644 index 000000000..dd69f52eb --- /dev/null +++ b/webrtc/system_wrappers/source/rw_lock_generic.h @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2011 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. + */ + +#ifndef WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_GENERIC_H_ +#define WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_GENERIC_H_ + +#include "webrtc/system_wrappers/interface/rw_lock_wrapper.h" +#include "webrtc/typedefs.h" + +namespace webrtc { + +class CriticalSectionWrapper; +class ConditionVariableWrapper; + +class RWLockGeneric : public RWLockWrapper { + public: + RWLockGeneric(); + virtual ~RWLockGeneric(); + + virtual void AcquireLockExclusive() OVERRIDE; + virtual void ReleaseLockExclusive() OVERRIDE; + + virtual void AcquireLockShared() OVERRIDE; + virtual void ReleaseLockShared() OVERRIDE; + + private: + CriticalSectionWrapper* critical_section_; + ConditionVariableWrapper* read_condition_; + ConditionVariableWrapper* write_condition_; + + int readers_active_; + bool writer_active_; + int readers_waiting_; + int writers_waiting_; +}; + +} // namespace webrtc + +#endif // WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_GENERIC_H_ diff --git a/webrtc/system_wrappers/source/rw_lock_posix.cc b/webrtc/system_wrappers/source/rw_lock_posix.cc new file mode 100644 index 000000000..cdcb7fb5b --- /dev/null +++ b/webrtc/system_wrappers/source/rw_lock_posix.cc @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2011 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 "webrtc/system_wrappers/source/rw_lock_posix.h" + +namespace webrtc { + +RWLockPosix::RWLockPosix() : lock_() { +} + +RWLockPosix::~RWLockPosix() { + pthread_rwlock_destroy(&lock_); +} + +RWLockPosix* RWLockPosix::Create() { + RWLockPosix* ret_val = new RWLockPosix(); + if (!ret_val->Init()) { + delete ret_val; + return NULL; + } + return ret_val; +} + +bool RWLockPosix::Init() { + return pthread_rwlock_init(&lock_, 0) == 0; +} + +void RWLockPosix::AcquireLockExclusive() { + pthread_rwlock_wrlock(&lock_); +} + +void RWLockPosix::ReleaseLockExclusive() { + pthread_rwlock_unlock(&lock_); +} + +void RWLockPosix::AcquireLockShared() { + pthread_rwlock_rdlock(&lock_); +} + +void RWLockPosix::ReleaseLockShared() { + pthread_rwlock_unlock(&lock_); +} + +} // namespace webrtc diff --git a/webrtc/system_wrappers/source/rw_lock_posix.h b/webrtc/system_wrappers/source/rw_lock_posix.h new file mode 100644 index 000000000..6ce2b2d7e --- /dev/null +++ b/webrtc/system_wrappers/source/rw_lock_posix.h @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2011 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. + */ + +#ifndef WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_POSIX_H_ +#define WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_POSIX_H_ + +#include "webrtc/system_wrappers/interface/rw_lock_wrapper.h" +#include "webrtc/typedefs.h" + +#include + +namespace webrtc { + +class RWLockPosix : public RWLockWrapper { + public: + static RWLockPosix* Create(); + virtual ~RWLockPosix(); + + virtual void AcquireLockExclusive() OVERRIDE; + virtual void ReleaseLockExclusive() OVERRIDE; + + virtual void AcquireLockShared() OVERRIDE; + virtual void ReleaseLockShared() OVERRIDE; + + private: + RWLockPosix(); + bool Init(); + + pthread_rwlock_t lock_; +}; + +} // namespace webrtc + +#endif // WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_POSIX_H_ diff --git a/webrtc/system_wrappers/source/rw_lock_win.cc b/webrtc/system_wrappers/source/rw_lock_win.cc new file mode 100644 index 000000000..aea74fa4a --- /dev/null +++ b/webrtc/system_wrappers/source/rw_lock_win.cc @@ -0,0 +1,97 @@ +/* + * Copyright (c) 2012 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 "webrtc/system_wrappers/source/rw_lock_win.h" + +#include "webrtc/system_wrappers/interface/trace.h" + +namespace webrtc { + +static bool native_rw_locks_supported = false; +static bool module_load_attempted = false; +static HMODULE library = NULL; + +typedef void (WINAPI* InitializeSRWLock)(PSRWLOCK); + +typedef void (WINAPI* AcquireSRWLockExclusive)(PSRWLOCK); +typedef void (WINAPI* ReleaseSRWLockExclusive)(PSRWLOCK); + +typedef void (WINAPI* AcquireSRWLockShared)(PSRWLOCK); +typedef void (WINAPI* ReleaseSRWLockShared)(PSRWLOCK); + +InitializeSRWLock initialize_srw_lock; +AcquireSRWLockExclusive acquire_srw_lock_exclusive; +AcquireSRWLockShared acquire_srw_lock_shared; +ReleaseSRWLockShared release_srw_lock_shared; +ReleaseSRWLockExclusive release_srw_lock_exclusive; + +RWLockWin::RWLockWin() { + initialize_srw_lock(&lock_); +} + +RWLockWin* RWLockWin::Create() { + if (!LoadModule()) { + return NULL; + } + return new RWLockWin(); +} + +void RWLockWin::AcquireLockExclusive() { + acquire_srw_lock_exclusive(&lock_); +} + +void RWLockWin::ReleaseLockExclusive() { + release_srw_lock_exclusive(&lock_); +} + +void RWLockWin::AcquireLockShared() { + acquire_srw_lock_shared(&lock_); +} + +void RWLockWin::ReleaseLockShared() { + release_srw_lock_shared(&lock_); +} + +bool RWLockWin::LoadModule() { + if (module_load_attempted) { + return native_rw_locks_supported; + } + module_load_attempted = true; + // Use native implementation if supported (i.e Vista+) + library = LoadLibrary(TEXT("Kernel32.dll")); + if (!library) { + return false; + } + WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, -1, "Loaded Kernel.dll"); + + initialize_srw_lock = + (InitializeSRWLock)GetProcAddress(library, "InitializeSRWLock"); + + acquire_srw_lock_exclusive = + (AcquireSRWLockExclusive)GetProcAddress(library, + "AcquireSRWLockExclusive"); + release_srw_lock_exclusive = + (ReleaseSRWLockExclusive)GetProcAddress(library, + "ReleaseSRWLockExclusive"); + acquire_srw_lock_shared = + (AcquireSRWLockShared)GetProcAddress(library, "AcquireSRWLockShared"); + release_srw_lock_shared = + (ReleaseSRWLockShared)GetProcAddress(library, "ReleaseSRWLockShared"); + + if (initialize_srw_lock && acquire_srw_lock_exclusive && + release_srw_lock_exclusive && acquire_srw_lock_shared && + release_srw_lock_shared) { + WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, -1, "Loaded Native RW Lock"); + native_rw_locks_supported = true; + } + return native_rw_locks_supported; +} + +} // namespace webrtc diff --git a/webrtc/system_wrappers/source/rw_lock_win.h b/webrtc/system_wrappers/source/rw_lock_win.h new file mode 100644 index 000000000..6f7cd3344 --- /dev/null +++ b/webrtc/system_wrappers/source/rw_lock_win.h @@ -0,0 +1,40 @@ +/* + * Copyright (c) 2012 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. + */ + +#ifndef WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_WIN_H_ +#define WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_WIN_H_ + +#include "webrtc/system_wrappers/interface/rw_lock_wrapper.h" + +#include + +namespace webrtc { + +class RWLockWin : public RWLockWrapper { + public: + static RWLockWin* Create(); + ~RWLockWin() {} + + virtual void AcquireLockExclusive(); + virtual void ReleaseLockExclusive(); + + virtual void AcquireLockShared(); + virtual void ReleaseLockShared(); + + private: + RWLockWin(); + static bool LoadModule(); + + SRWLOCK lock_; +}; + +} // namespace webrtc + +#endif // WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_WIN_H_ diff --git a/webrtc/system_wrappers/system_wrappers.gyp b/webrtc/system_wrappers/system_wrappers.gyp index 3af2e0276..3af889787 100644 --- a/webrtc/system_wrappers/system_wrappers.gyp +++ b/webrtc/system_wrappers/system_wrappers.gyp @@ -86,6 +86,12 @@ 'source/logging.cc', 'source/rtp_to_ntp.cc', 'source/rw_lock.cc', + 'source/rw_lock_generic.cc', + 'source/rw_lock_generic.h', + 'source/rw_lock_posix.cc', + 'source/rw_lock_posix.h', + 'source/rw_lock_win.cc', + 'source/rw_lock_win.h', 'source/set_thread_name_win.h', 'source/sleep.cc', 'source/sort.cc',