Some style fixes.

Refactors the rw lock implementation.

BUG=N/A

Review URL: https://webrtc-codereview.appspot.com/797004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@2822 4adac7df-926f-26a2-2b94-8c16560cd09d

src/system_wrappers/interface/rw_lock_wrapper.h

@@ -16,20 +16,18 @@
 // functionality and will therefore have worse performance.
 
 namespace webrtc {
+
 class RWLockWrapper
 {
 public:
     static RWLockWrapper* CreateRWLock();
-    virtual ~RWLockWrapper();
+    virtual ~RWLockWrapper() {}
 
     virtual void AcquireLockExclusive() = 0;
     virtual void ReleaseLockExclusive() = 0;
 
     virtual void AcquireLockShared() = 0;
     virtual void ReleaseLockShared() = 0;
-
-protected:
-    virtual int Init() = 0;
 };
 
 // RAII extensions of the RW lock. Prevents Acquire/Release missmatches and
@@ -71,6 +69,7 @@ public:
 private:
     RWLockWrapper& _rwLock;
 };
+
 } // namespace webrtc
 
 #endif // WEBRTC_SYSTEM_WRAPPERS_INTERFACE_RW_LOCK_WRAPPER_H_

src/system_wrappers/source/rw_lock.cc

@@ -13,29 +13,25 @@
 #include <assert.h>
 
 #if defined(_WIN32)
+#include "rw_lock_generic.h"
 #include "rw_lock_win.h"
 #else
 #include "rw_lock_posix.h"
 #endif
 
 namespace webrtc {
-RWLockWrapper* RWLockWrapper::CreateRWLock()
-{
+
+RWLockWrapper* RWLockWrapper::CreateRWLock() {
 #ifdef _WIN32
-    RWLockWrapper* lock =  new RWLockWindows();
-#else
-    RWLockWrapper* lock =  new RWLockPosix();
-#endif
-    if(lock->Init() != 0)
-    {
-        delete lock;
-        assert(false);
-        return NULL;
-    }
+  // Native implementation is faster, so use that if available.
+  RWLockWrapper* lock = RWLockWin::Create();
+  if (lock) {
     return lock;
   }
-
-RWLockWrapper::~RWLockWrapper()
-{
+  return new RWLockGeneric();
+#else
+  return RWLockPosix::Create();
+#endif
 }
+
 }  // namespace webrtc

src/system_wrappers/source/rw_lock_generic.cc

@@ -8,99 +8,70 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
-#include "rw_lock_generic.h"
+#include "system_wrappers/source/rw_lock_generic.h"
 
-#include "condition_variable_wrapper.h"
-#include "critical_section_wrapper.h"
+#include "system_wrappers/interface/condition_variable_wrapper.h"
+#include "system_wrappers/interface/critical_section_wrapper.h"
 
 namespace webrtc {
-RWLockWrapperGeneric::RWLockWrapperGeneric()
-    : _readersActive(0),
-      _writerActive(false),
-      _readersWaiting(0),
-      _writersWaiting(0)
-{
-    _critSectPtr  = CriticalSectionWrapper::CreateCriticalSection();
-    _readCondPtr  = ConditionVariableWrapper::CreateConditionVariable();
-    _writeCondPtr = ConditionVariableWrapper::CreateConditionVariable();
+
+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();
 }
 
-RWLockWrapperGeneric::~RWLockWrapperGeneric()
-{
-    delete _writeCondPtr;
-    delete _readCondPtr;
-    delete _critSectPtr;
+RWLockGeneric::~RWLockGeneric() {
+  delete write_condition_;
+  delete read_condition_;
+  delete critical_section_;
 }
 
-int RWLockWrapperGeneric::Init()
-{
-    return 0;
+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 RWLockWrapperGeneric::AcquireLockExclusive()
-{
-    _critSectPtr->Enter();
-
-    if (_writerActive || _readersActive > 0)
-    {
-        ++_writersWaiting;
-
-        while (_writerActive || _readersActive > 0)
-        {
-            _writeCondPtr->SleepCS(*_critSectPtr);
+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();
+  }
 }
 
-        --_writersWaiting;
+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_);
     }
-    _writerActive = true;
-    _critSectPtr->Leave();
+    --readers_waiting_;
+  }
+  ++readers_active_;
 }
 
-void RWLockWrapperGeneric::ReleaseLockExclusive()
-{
-    _critSectPtr->Enter();
-
-    _writerActive = false;
-
-    if (_writersWaiting > 0)
-    {
-        _writeCondPtr->Wake();
-
-    }else if (_readersWaiting > 0)
-    {
-        _readCondPtr->WakeAll();
+void RWLockGeneric::ReleaseLockShared() {
+  CriticalSectionScoped cs(critical_section_);
+  --readers_active_;
+  if (readers_active_ == 0 && writers_waiting_ > 0) {
+    write_condition_->Wake();
   }
-    _critSectPtr->Leave();
 }
 
-void RWLockWrapperGeneric::AcquireLockShared()
-{
-    _critSectPtr->Enter();
-
-    if (_writerActive || _writersWaiting > 0)
-    {
-        ++_readersWaiting;
-
-        while (_writerActive || _writersWaiting > 0)
-        {
-            _readCondPtr->SleepCS(*_critSectPtr);
-        }
-        --_readersWaiting;
-    }
-    ++_readersActive;
-    _critSectPtr->Leave();
-}
-
-void RWLockWrapperGeneric::ReleaseLockShared()
-{
-    _critSectPtr->Enter();
-
-    --_readersActive;
-
-    if (_readersActive == 0 && _writersWaiting > 0)
-    {
-        _writeCondPtr->Wake();
-    }
-    _critSectPtr->Leave();
-}
 }  // namespace webrtc

src/system_wrappers/source/rw_lock_generic.h

@@ -11,17 +11,17 @@
 #ifndef WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_GENERIC_H_
 #define WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_GENERIC_H_
 
-#include "rw_lock_wrapper.h"
+#include "system_wrappers/interface/rw_lock_wrapper.h"
 
 namespace webrtc {
+
 class CriticalSectionWrapper;
 class ConditionVariableWrapper;
 
-class RWLockWrapperGeneric : public RWLockWrapper
-{
+class RWLockGeneric : public RWLockWrapper {
 public:
-    RWLockWrapperGeneric();
-    virtual ~RWLockWrapperGeneric();
+    RWLockGeneric();
+    virtual ~RWLockGeneric();
 
     virtual void AcquireLockExclusive();
     virtual void ReleaseLockExclusive();
@@ -29,18 +29,17 @@ public:
     virtual void AcquireLockShared();
     virtual void ReleaseLockShared();
 
-protected:
-    virtual int Init();
-
 private:
-    CriticalSectionWrapper*   _critSectPtr;
-    ConditionVariableWrapper* _readCondPtr;
-    ConditionVariableWrapper* _writeCondPtr;
+    CriticalSectionWrapper* critical_section_;
+    ConditionVariableWrapper* read_condition_;
+    ConditionVariableWrapper* write_condition_;
 
-    int  _readersActive;
-    bool _writerActive;
-    int  _readersWaiting;
-    int  _writersWaiting;
+    int readers_active_;
+    bool writer_active_;
+    int readers_waiting_;
+    int writers_waiting_;
 };
+
 }  // namespace webrtc
+
 #endif  // WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_GENERIC_H_

src/system_wrappers/source/rw_lock_posix.cc

@@ -8,40 +8,44 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
-#include "rw_lock_posix.h"
+#include "system_wrappers/source/rw_lock_posix.h"
 
 namespace webrtc {
-RWLockPosix::RWLockPosix() : _lock()
-{
+
+RWLockPosix::RWLockPosix() : _lock() {
 }
 
-RWLockPosix::~RWLockPosix()
-{
+RWLockPosix::~RWLockPosix() {
   pthread_rwlock_destroy(&_lock);
 }
 
-int RWLockPosix::Init()
-{
-    return pthread_rwlock_init(&_lock, 0);
+RWLockPosix* RWLockPosix::Create() {
+  RWLockPosix* ret_val = new RWLockPosix();
+  if (!ret_val->Init()) {
+    delete ret_val;
+    return NULL;
+  }
+  return ret_val;
 }
 
-void RWLockPosix::AcquireLockExclusive()
-{
+bool RWLockPosix::Init() {
+  return pthread_rwlock_init(&_lock, 0) == 0;
+}
+
+void RWLockPosix::AcquireLockExclusive() {
   pthread_rwlock_wrlock(&_lock);
 }
 
-void RWLockPosix::ReleaseLockExclusive()
-{
+void RWLockPosix::ReleaseLockExclusive() {
   pthread_rwlock_unlock(&_lock);
 }
 
-void RWLockPosix::AcquireLockShared()
-{
+void RWLockPosix::AcquireLockShared() {
   pthread_rwlock_rdlock(&_lock);
 }
 
-void RWLockPosix::ReleaseLockShared()
-{
+void RWLockPosix::ReleaseLockShared() {
   pthread_rwlock_unlock(&_lock);
 }
+
 }  // namespace webrtc

src/system_wrappers/source/rw_lock_posix.h

@@ -11,15 +11,16 @@
 #ifndef WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_POSIX_H_
 #define WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_POSIX_H_
 
-#include "rw_lock_wrapper.h"
+#include "system_wrappers/interface/rw_lock_wrapper.h"
 
 #include <pthread.h>
 
 namespace webrtc {
+
 class RWLockPosix : public RWLockWrapper
 {
 public:
-    RWLockPosix();
+    static RWLockPosix* Create();
     virtual ~RWLockPosix();
 
     virtual void AcquireLockExclusive();
@@ -28,12 +29,13 @@ public:
     virtual void AcquireLockShared();
     virtual void ReleaseLockShared();
 
-protected:
-    virtual int Init();
-
 private:
+    RWLockPosix();
+    bool Init();
+
     pthread_rwlock_t _lock;
 };
+
 }  // namespace webrtc
 
 #endif  // WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_POSIX_H_

src/system_wrappers/source/rw_lock_win.cc

@@ -1,5 +1,5 @@
 /*
- *  Copyright (c) 2011 The WebRTC project authors. All Rights Reserved.
+ *  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
@@ -8,179 +8,90 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
-#include "rw_lock_win.h"
+#include "system_wrappers/source/rw_lock_win.h"
 
-#include "critical_section_wrapper.h"
-#include "condition_variable_wrapper.h"
 #include "trace.h"
 
-// TODO (hellner) why not just use the rw_lock_generic.cc solution if
-//                           native is not supported? Unnecessary redundancy!
-
 namespace webrtc {
-bool RWLockWindows::_winSupportRWLockPrimitive = false;
+
+static bool native_rw_locks_supported = false;
+static bool module_load_attempted = false;
 static HMODULE library = NULL;
 
-PInitializeSRWLock       _PInitializeSRWLock;
-PAcquireSRWLockExclusive _PAcquireSRWLockExclusive;
-PAcquireSRWLockShared    _PAcquireSRWLockShared;
-PReleaseSRWLockShared    _PReleaseSRWLockShared;
-PReleaseSRWLockExclusive _PReleaseSRWLockExclusive;
+typedef void (WINAPI *InitializeSRWLock)(PSRWLOCK);
 
-RWLockWindows::RWLockWindows()
-    : _critSectPtr(NULL),
-      _readCondPtr(NULL),
-      _writeCondPtr(NULL),
-      _readersActive(0),
-      _writerActive(false),
-      _readersWaiting(0),
-      _writersWaiting(0)
-{
+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_);
 }
 
-RWLockWindows::~RWLockWindows()
-{
-    delete _writeCondPtr;
-    delete _readCondPtr;
-    delete _critSectPtr;
+RWLockWin* RWLockWin::Create() {
+  if (!LoadModule()) {
+    return NULL;
+  }
+  return new RWLockWin();
 }
 
-int RWLockWindows::Init()
-{
-    if(!library)
-    {
+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)
-        {
-            WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, -1,
-                         "Loaded Kernel.dll");
+  if (!library) {
+    return false;
+  }
+  WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, -1, "Loaded Kernel.dll");
 
-            _PInitializeSRWLock =
-                (PInitializeSRWLock)GetProcAddress(
-                    library,
-                    "InitializeSRWLock");
+  initialize_srw_lock =
+      (InitializeSRWLock)GetProcAddress(library, "InitializeSRWLock");
 
-            _PAcquireSRWLockExclusive =
-               (PAcquireSRWLockExclusive)GetProcAddress(
-                   library,
+  acquire_srw_lock_exclusive =
+      (AcquireSRWLockExclusive)GetProcAddress(library,
                                               "AcquireSRWLockExclusive");
-            _PReleaseSRWLockExclusive =
-                (PReleaseSRWLockExclusive)GetProcAddress(
-                    library,
+  release_srw_lock_exclusive =
+      (ReleaseSRWLockExclusive)GetProcAddress(library,
                                               "ReleaseSRWLockExclusive");
-            _PAcquireSRWLockShared =
-                (PAcquireSRWLockShared)GetProcAddress(
-                    library,
-                    "AcquireSRWLockShared");
-            _PReleaseSRWLockShared =
-                (PReleaseSRWLockShared)GetProcAddress(
-                    library,
-                    "ReleaseSRWLockShared");
+  acquire_srw_lock_shared =
+      (AcquireSRWLockShared)GetProcAddress(library, "AcquireSRWLockShared");
+  release_srw_lock_shared =
+      (ReleaseSRWLockShared)GetProcAddress(library, "ReleaseSRWLockShared");
 
-            if( _PInitializeSRWLock &&
-                _PAcquireSRWLockExclusive &&
-                _PReleaseSRWLockExclusive &&
-                _PAcquireSRWLockShared &&
-                _PReleaseSRWLockShared )
-            {
-                WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, -1,
-                            "Loaded Simple RW Lock");
-                _winSupportRWLockPrimitive = true;
+  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;
   }
-        }
-    }
-    if(_winSupportRWLockPrimitive)
-    {
-        _PInitializeSRWLock(&_lock);
-    } else {
-        _critSectPtr  = CriticalSectionWrapper::CreateCriticalSection();
-        _readCondPtr  = ConditionVariableWrapper::CreateConditionVariable();
-        _writeCondPtr = ConditionVariableWrapper::CreateConditionVariable();
-    }
-    return 0;
+  return native_rw_locks_supported;
 }
 
-void RWLockWindows::AcquireLockExclusive()
-{
-    if (_winSupportRWLockPrimitive)
-    {
-        _PAcquireSRWLockExclusive(&_lock);
-    } else {
-        _critSectPtr->Enter();
-
-        if (_writerActive || _readersActive > 0)
-        {
-            ++_writersWaiting;
-            while (_writerActive || _readersActive > 0)
-            {
-                _writeCondPtr->SleepCS(*_critSectPtr);
-            }
-            --_writersWaiting;
-        }
-        _writerActive = true;
-        _critSectPtr->Leave();
-    }
-}
-
-void RWLockWindows::ReleaseLockExclusive()
-{
-    if(_winSupportRWLockPrimitive)
-    {
-        _PReleaseSRWLockExclusive(&_lock);
-    } else {
-        _critSectPtr->Enter();
-        _writerActive = false;
-        if (_writersWaiting > 0)
-        {
-            _writeCondPtr->Wake();
-
-        }else if (_readersWaiting > 0) {
-            _readCondPtr->WakeAll();
-        }
-        _critSectPtr->Leave();
-    }
-}
-
-void RWLockWindows::AcquireLockShared()
-{
-    if(_winSupportRWLockPrimitive)
-    {
-        _PAcquireSRWLockShared(&_lock);
-    } else
-    {
-        _critSectPtr->Enter();
-        if (_writerActive || _writersWaiting > 0)
-        {
-            ++_readersWaiting;
-
-            while (_writerActive || _writersWaiting > 0)
-            {
-                _readCondPtr->SleepCS(*_critSectPtr);
-            }
-            --_readersWaiting;
-        }
-        ++_readersActive;
-        _critSectPtr->Leave();
-    }
-}
-
-void RWLockWindows::ReleaseLockShared()
-{
-    if(_winSupportRWLockPrimitive)
-    {
-        _PReleaseSRWLockShared(&_lock);
-    } else
-    {
-        _critSectPtr->Enter();
-
-        --_readersActive;
-
-        if (_readersActive == 0 && _writersWaiting > 0)
-        {
-            _writeCondPtr->Wake();
-        }
-        _critSectPtr->Leave();
-    }
-}
 }  // namespace webrtc

src/system_wrappers/source/rw_lock_win.h

@@ -1,5 +1,5 @@
 /*
- *  Copyright (c) 2011 The WebRTC project authors. All Rights Reserved.
+ *  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
@@ -8,39 +8,19 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
-#ifndef WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_WINDOWS_H_
-#define WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_WINDOWS_H_
+#ifndef WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_WIN__H_
+#define WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_WIN__H_
 
-#include "rw_lock_wrapper.h"
+#include "system_wrappers/interface/rw_lock_wrapper.h"
 
 #include <Windows.h>
 
-#if !defined(RTL_SRWLOCK_INIT)
-    typedef struct _RTL_SRWLOCK
-    {
-        void* Ptr;
-    } RTL_SRWLOCK, *PRTL_SRWLOCK;
-    typedef RTL_SRWLOCK SRWLOCK, *PSRWLOCK;
-#endif
-
 namespace webrtc {
-class CriticalSectionWrapper;
-class ConditionVariableWrapper;
 
-typedef void (WINAPI *PInitializeSRWLock)(PSRWLOCK);
-
-typedef void (WINAPI *PAcquireSRWLockExclusive)(PSRWLOCK);
-typedef void (WINAPI *PReleaseSRWLockExclusive)(PSRWLOCK);
-
-typedef void (WINAPI *PAcquireSRWLockShared)(PSRWLOCK);
-typedef void (WINAPI *PReleaseSRWLockShared)(PSRWLOCK);
-
-
-class RWLockWindows :public RWLockWrapper
-{
+class RWLockWin : public RWLockWrapper {
  public:
-    RWLockWindows();
-    virtual ~RWLockWindows();
+  static RWLockWin* Create();
+  ~RWLockWin() {}
 
   virtual void AcquireLockExclusive();
   virtual void ReleaseLockExclusive();
@@ -48,24 +28,13 @@ public:
   virtual void AcquireLockShared();
   virtual void ReleaseLockShared();
 
-protected:
-    virtual int Init();
-
  private:
-    // For native implementation.
-    static bool _winSupportRWLockPrimitive;
-    SRWLOCK     _lock;
+  RWLockWin();
+  static bool LoadModule();
 
-    // Genric implementation, fallback if native is not supported.
-    CriticalSectionWrapper*   _critSectPtr;
-    ConditionVariableWrapper* _readCondPtr;
-    ConditionVariableWrapper* _writeCondPtr;
-
-    int  _readersActive;
-    bool _writerActive;
-    int  _readersWaiting;
-    int  _writersWaiting;
+  SRWLOCK lock_;
 };
+
 }  // namespace webrtc
 
-#endif // WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_WINDOWS_H_
+#endif  // WEBRTC_SYSTEM_WRAPPERS_SOURCE_RW_LOCK_WIN__H_

src/system_wrappers/source/system_wrappers.gyp

@@ -85,6 +85,8 @@
         'list_no_stl.cc',
         'map.cc',
         'rw_lock.cc',
+        'rw_lock_generic.cc',
+        'rw_lock_generic.h',
         'rw_lock_posix.cc',
         'rw_lock_posix.h',
         'rw_lock_win.cc',